KAFKA-2390-followup; add unit test for OffsetOutOfRange exception#239
Closed
lindong28 wants to merge 5 commits into
Closed
KAFKA-2390-followup; add unit test for OffsetOutOfRange exception#239lindong28 wants to merge 5 commits into
lindong28 wants to merge 5 commits into
Conversation
|
kafka-trunk-git-pr #532 FAILURE |
Contributor
|
@lindong28 Why not include this test in FetcherTest? Also, do you think it's worthwhile adding a test case which checks that the out of range error is ignored when the user seeks to a different offset? |
Member
Author
|
@hachikuji I couldn't include this test in FetcherTest, because to test OffsetOutOfRange exception the defaultResetPolicy needs to be NONE. But the defaultResetPolicy can't be changed after setup. Sure, let me add another test to check that the out of range error is ignored when the user seeks to a different offset. |
|
kafka-trunk-git-pr #533 SUCCESS |
Contributor
|
@lindong28 Not sure that problem calls for a new class. An easy way would be to create a factory method in FetcherTest which each test uses to construct a new Fetcher instance. Up to you, but it's nice to keep related tests organized together. |
|
kafka-trunk-git-pr #534 FAILURE |
Member
Author
|
@hachikuji I agree that nice to keep related tests organized together. I can implement that factory class to be used across test cases. Another alternative is to add method setOffsetResetStrategy() in class SubscriptionState. That looks OK to me and would make things much easier here. What do you think? |
Member
Author
|
@hachikuji I have moved the test cases to FetcherTest. Please let me know if it looks good. Thank you. |
Contributor
There was a problem hiding this comment.
Might be no need to make this a field since it's only used in two test cases.
Contributor
There was a problem hiding this comment.
Sounds OK to me since it is used more than once.
Contributor
|
@lindong28 One minor comment. Overall, LGTM. |
|
kafka-trunk-git-pr #537 SUCCESS |
Member
Author
|
Thanks @hachikuji @guozhangwang for your quick review. |
jsancio
pushed a commit
to jsancio/kafka
that referenced
this pull request
Aug 6, 2019
As we are stopping brokers in these tests, there is a timing issue where the TierTopicManager doesn't manage to create the tier topic and doesn't become ready in time to archive the segments we produce. By the time it tries to create the tier topic, there are not enough brokers for the replication factor, meaning that all tiering is blocked. Also improve logging when tier topic creation fails.
efeg
added a commit
to efeg/kafka
that referenced
this pull request
Jan 29, 2020
…o initialize zkUtils. (apache#239) - Fix executor state being stuck in EXECUTION_STARTED due to failures to initialize zkUtils. - Use a single zkUtils within the Executor.
wyuka
added a commit
to wyuka/kafka
that referenced
this pull request
Jan 13, 2022
…c storage. (apache#239) Kafka JIRA: KAFKA-9555 Source commit: satishd/kafka@25f9120 Summary: - Introduced TopicBasedRemoteLogMetadataManagerHarness which takes care of bringing up a Kafka cluster and create remote log metadata topic and initializes TopicBasedRemoteLogMetadataManager. - Added TopicBasedRemoteLogMetadataManager to run the existing tests by making the existing RemoteLogMetadataManagerTest parameterized. - Added transformation of TopicIdPartition into bytes with custom hashing. - Skipping the events which are not currently assigned for the ConsumerTask. Added file based remote log metadata snapshots: - Added TopicBasedRemoteLogMetadataManagerRestartTest and fixed issues in reloading the snapshots. - TopicBasedRemoteLogMetadataManagerRestartTest is about verifying: * load the earlier saved snapshots after restart * check the entries are available * start the consumer and add more metadata entries * check the newly addded entries and loaded entries are available - Fixed a few issues in snapshot read/write/load and added tests. This is part of tiered storage KIP-405 efforts. Co-authored-by: Satish Duggana <satishd@apache.org>
wyuka
added a commit
to wyuka/kafka
that referenced
this pull request
Mar 4, 2022
…c storage. (apache#239) Kafka JIRA: KAFKA-9555 Source commit: satishd/kafka@25f9120 Summary: - Introduced TopicBasedRemoteLogMetadataManagerHarness which takes care of bringing up a Kafka cluster and create remote log metadata topic and initializes TopicBasedRemoteLogMetadataManager. - Added TopicBasedRemoteLogMetadataManager to run the existing tests by making the existing RemoteLogMetadataManagerTest parameterized. - Added transformation of TopicIdPartition into bytes with custom hashing. - Skipping the events which are not currently assigned for the ConsumerTask. Added file based remote log metadata snapshots: - Added TopicBasedRemoteLogMetadataManagerRestartTest and fixed issues in reloading the snapshots. - TopicBasedRemoteLogMetadataManagerRestartTest is about verifying: * load the earlier saved snapshots after restart * check the entries are available * start the consumer and add more metadata entries * check the newly addded entries and loaded entries are available - Fixed a few issues in snapshot read/write/load and added tests. This is part of tiered storage KIP-405 efforts. Co-authored-by: Satish Duggana <satishd@apache.org>
wyuka
added a commit
to wyuka/kafka
that referenced
this pull request
Mar 28, 2022
…c storage (apache#239) Kafka JIRA: KAFKA-9555 Source commit: satishd/kafka@25f9120 Summary: - Introduced TopicBasedRemoteLogMetadataManagerHarness which takes care of bringing up a Kafka cluster and create remote log metadata topic and initializes TopicBasedRemoteLogMetadataManager. - Added TopicBasedRemoteLogMetadataManager to run the existing tests by making the existing RemoteLogMetadataManagerTest parameterized. - Added transformation of TopicIdPartition into bytes with custom hashing. - Skipping the events which are not currently assigned for the ConsumerTask. Added file based remote log metadata snapshots: - Added TopicBasedRemoteLogMetadataManagerRestartTest and fixed issues in reloading the snapshots. - TopicBasedRemoteLogMetadataManagerRestartTest is about verifying: * load the earlier saved snapshots after restart * check the entries are available * start the consumer and add more metadata entries * check the newly addded entries and loaded entries are available - Fixed a few issues in snapshot read/write/load and added tests. This is part of tiered storage KIP-405 efforts. Co-authored-by: Satish Duggana <satishd@apache.org>
wyuka
added a commit
to wyuka/kafka
that referenced
this pull request
Jun 16, 2022
…c storage (apache#239) Kafka JIRA: KAFKA-9555 Source commit: satishd/kafka@25f9120 Summary: - Introduced TopicBasedRemoteLogMetadataManagerHarness which takes care of bringing up a Kafka cluster and create remote log metadata topic and initializes TopicBasedRemoteLogMetadataManager. - Added TopicBasedRemoteLogMetadataManager to run the existing tests by making the existing RemoteLogMetadataManagerTest parameterized. - Added transformation of TopicIdPartition into bytes with custom hashing. - Skipping the events which are not currently assigned for the ConsumerTask. Added file based remote log metadata snapshots: - Added TopicBasedRemoteLogMetadataManagerRestartTest and fixed issues in reloading the snapshots. - TopicBasedRemoteLogMetadataManagerRestartTest is about verifying: * load the earlier saved snapshots after restart * check the entries are available * start the consumer and add more metadata entries * check the newly addded entries and loaded entries are available - Fixed a few issues in snapshot read/write/load and added tests. This is part of tiered storage KIP-405 efforts. Co-authored-by: Satish Duggana <satishd@apache.org>
mdedetrich
pushed a commit
to aiven/kafka
that referenced
this pull request
Nov 2, 2022
…c storage (apache#239) Kafka JIRA: KAFKA-9555 Source commit: satishd/kafka@25f9120 Summary: - Introduced TopicBasedRemoteLogMetadataManagerHarness which takes care of bringing up a Kafka cluster and create remote log metadata topic and initializes TopicBasedRemoteLogMetadataManager. - Added TopicBasedRemoteLogMetadataManager to run the existing tests by making the existing RemoteLogMetadataManagerTest parameterized. - Added transformation of TopicIdPartition into bytes with custom hashing. - Skipping the events which are not currently assigned for the ConsumerTask. Added file based remote log metadata snapshots: - Added TopicBasedRemoteLogMetadataManagerRestartTest and fixed issues in reloading the snapshots. - TopicBasedRemoteLogMetadataManagerRestartTest is about verifying: * load the earlier saved snapshots after restart * check the entries are available * start the consumer and add more metadata entries * check the newly addded entries and loaded entries are available - Fixed a few issues in snapshot read/write/load and added tests. This is part of tiered storage KIP-405 efforts. Co-authored-by: Satish Duggana <satishd@apache.org>
mdedetrich
pushed a commit
to aiven/kafka
that referenced
this pull request
Nov 24, 2022
…c storage (apache#239) Kafka JIRA: KAFKA-9555 Source commit: satishd/kafka@25f9120 Summary: - Introduced TopicBasedRemoteLogMetadataManagerHarness which takes care of bringing up a Kafka cluster and create remote log metadata topic and initializes TopicBasedRemoteLogMetadataManager. - Added TopicBasedRemoteLogMetadataManager to run the existing tests by making the existing RemoteLogMetadataManagerTest parameterized. - Added transformation of TopicIdPartition into bytes with custom hashing. - Skipping the events which are not currently assigned for the ConsumerTask. Added file based remote log metadata snapshots: - Added TopicBasedRemoteLogMetadataManagerRestartTest and fixed issues in reloading the snapshots. - TopicBasedRemoteLogMetadataManagerRestartTest is about verifying: * load the earlier saved snapshots after restart * check the entries are available * start the consumer and add more metadata entries * check the newly addded entries and loaded entries are available - Fixed a few issues in snapshot read/write/load and added tests. This is part of tiered storage KIP-405 efforts. Co-authored-by: Satish Duggana <satishd@apache.org>
udaynpusa
pushed a commit
to mapr/kafka
that referenced
this pull request
Jan 30, 2024
davide-armand
pushed a commit
to aiven/kafka
that referenced
this pull request
Dec 1, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.