Fix to metadata cache expiration on full metadata refresh #4677
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emasab
force-pushed
the
dev_fix_metadata_cache_cleared_full_request
branch
from
fa6ea32 to
6f3c1b5
Compare
emasab
force-pushed
the
dev_metadata_cache_topic_id
branch
from
4d7eab9 to
8ea3b9e
Compare
emasab
force-pushed
the
dev_fix_metadata_cache_cleared_full_request
branch
2 times, most recently
from
b27ccc6 to
d4d40c1
Compare
emasab
force-pushed
the
dev_fix_metadata_cache_cleared_full_request
branch
from
d4d40c1 to
d9540da
Compare
pranavrth
requested changes
Apr 11, 2024
| rd_kafka_t *rk, | ||
| const rd_kafka_metadata_topic_t *mdt, | ||
| const rd_kafka_metadata_topic_internal_t *mdit, | ||
| rd_bool_t propagate, | ||
| rd_bool_t include_metadata, | ||
| rd_kafka_metadata_broker_internal_t *brokers, | ||
| size_t broker_cnt); | ||
| size_t broker_cnt, | ||
| rd_bool_t only_existing); |
There was a problem hiding this comment.
Better name maybe update_only_existing and let's explain what it means in the doc.
There was a problem hiding this comment.
If you search for only_ the code convention in this repo is to avoid repeating the action in variable names, for example in rd_kafka_buf_write_topic_partitions we have only_invalid_offsets not write_only_invalid_offsets.
Okey for the doc
src/rdkafka_metadata_cache.c
Outdated
| rkmce = rd_kafka_metadata_cache_find(rk, mdt->topic, 0); | ||
| if (!rkmce) | ||
| return 0; | ||
| } else if (unlikely(!mdt->topic)) { |
There was a problem hiding this comment.
This is not checking only_existing part.
on full metadata refresh Metadata cache was cleared on full metadata refresh, leading to unnecessary refreshes and occasional `UNKNOWN_TOPIC_OR_PART` errors. Solved by updating cache for existing or hinted entries instead of clearing them. Happening since 2.1.0
emasab
force-pushed
the
dev_fix_metadata_cache_cleared_full_request
branch
from
d9540da to
a159193
Compare
pranavrth
approved these changes
Apr 14, 2024
| @@ -35,15 +35,6 @@ def read_scenario_conf(scenario): | |||
| return parser.load(f) | |||
|
|
|||
|
|
|||
| # FIXME: merge in trivup | |||
| class KafkaBrokerApp(KafkaBrokerAppOrig): | |||
There was a problem hiding this comment.
Yeah, it was added back during a rebase
| int main_0146_metadata_mock(int argc, char **argv) { | ||
| TEST_SKIP_MOCK_CLUSTER(0); | ||
|
|
||
| do_test_metadata_persists_in_cache("range"); |
There was a problem hiding this comment.
The difference can be because of taking the eager path instead of the incremental one, but roundrobin also takes the eager path, as "range", so it's not important for this test.
7 tasks
emasab
added a commit
that referenced
this pull request
Apr 15, 2024
Metadata cache was cleared on full metadata refresh, leading to unnecessary refreshes and occasional `UNKNOWN_TOPIC_OR_PART` errors. Solved by updating cache for existing or hinted entries instead of clearing them. Happening since 2.1.0
emasab
added a commit
that referenced
this pull request
Apr 18, 2024
Metadata cache was cleared on full metadata refresh, leading to unnecessary refreshes and occasional `UNKNOWN_TOPIC_OR_PART` errors. Solved by updating cache for existing or hinted entries instead of clearing them. Happening since 2.1.0
anchitj
pushed a commit
that referenced
this pull request
Jun 10, 2024
Metadata cache was cleared on full metadata refresh, leading to unnecessary refreshes and occasional `UNKNOWN_TOPIC_OR_PART` errors. Solved by updating cache for existing or hinted entries instead of clearing them. Happening since 2.1.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Metadata cache was cleared on full metadata
refresh, leading to unnecessary refreshes and
occasional
UNKNOWN_TOPIC_OR_PARTerrors.Solved by updating cache for existing or
hinted entries instead of clearing them.
Happening since 2.1.0