Prevent improper error logging during worker shutdown #1257
+23
−5
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
…er error logging during worker shutdown
Please keep a short title for the commit but move the explanation and details to the description section |
zachjhum
changed the title
| @@ -377,13 +380,27 @@ record = Record.builder().data(createByteBufferWithSize(1024)).build(); | |||
| @Test(expected = IllegalStateException.class) | |||
| public void testGetNextRecordsWithoutStarting() { | |||
There was a problem hiding this comment.
oh can we please change the test name to reflect what it is doing : testSubscribeWithouStarting
| @@ -377,13 +380,27 @@ record = Record.builder().data(createByteBufferWithSize(1024)).build(); | |||
| @Test(expected = IllegalStateException.class) | |||
| public void testGetNextRecordsWithoutStarting() { | |||
| verify(executorService, never()).execute(any()); | |||
| getRecordsCache.drainQueueForRequests(); | |||
| Subscriber<RecordsRetrieved> mockSubscriber = mock(Subscriber.class); | |||
| getRecordsCache.subscribe(mockSubscriber); | |||
| } | |||
|
|
|||
| @Test(expected = IllegalStateException.class) | |||
| public void testCallAfterShutdown() { | |||
There was a problem hiding this comment.
same here, maybe testRequestRecordsOnSubscriptionAfterShutdown
|
Tested with a KCL application. On subscribe (which happens during startup) or lease loss, did not see the exception logged. |
akidambisrinivasan
approved these changes
Feb 21, 2024
akidambisrinivasan
pushed a commit
to akidambisrinivasan/amazon-kinesis-client
that referenced
this pull request
Apr 29, 2024
* Move throwOnIllegalState call to drain queue method to prevent improper error logging during worker shutdown * Fix unit tests that expected IllegalStateException thrown * Changed names of unit tests to reflect new behavior
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.
This fix deals with a customer issue where the KCL logs an IllegalStateException during even a graceful worker shutdown. See #914