Fix marking individual deletes as dirty #9732
Merged
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
When we mark cursors as dirty, we aren't marking when individual acks cause a dirty cursor. This results in cursors not being flushed and causing redelivery. This one line fix will ensure we mark the cursor as dirty in this situation as well
codelipenghui
approved these changes
Feb 26, 2021
codelipenghui
added
release/2.7.1
release/2.6.4
type/bug
|
cc @merlimat FYI |
zymap
approved these changes
Feb 26, 2021
merlimat
approved these changes
Feb 26, 2021
jiazhai
approved these changes
Feb 26, 2021
|
/pulsarbot run-failure-checks |
lhotari
reviewed
Feb 26, 2021
| assertNotEquals(dirtyCursor.getMarkDeletedPosition(), positions.get(positions.size() - 1)); | ||
|
|
||
| // Give chance to the flush to be automatically triggered. | ||
| Thread.sleep(3000); |
There was a problem hiding this comment.
Using Thread.sleep in test could lead to flaky tests. Is there a possibility to use Awaitility to poll for some condition or assertion? Similar logic with retrying using Awaitility:
Awaitility.await()
// Give chance to the flush to be automatically triggered.
.pollDelay(Duration.ofMillis(3000))
.untilAsserted(() -> {
// Abruptly re-open the managed ledger without graceful close
ManagedLedgerFactory factory2 = new ManagedLedgerFactoryImpl(bkc, bkc.getZkHandle());
try {
ManagedLedger ledger2 = factory2.open("testFlushCursorAfterInactivity", config);
ManagedCursor c2 = ledger2.openCursor("c");
assertEquals(c2.getMarkDeletedPosition(), positions.get(positions.size() - 1));
} finally {
factory2.shutdown();
}
});
By default, Awaitility will retry up to 10 seconds.
eolivelli
approved these changes
Feb 26, 2021
aahmed-se
approved these changes
Feb 26, 2021
|
Please remove the Thread.sleep before merging this patch |
|
@eolivelli @lhotari fix the test, as well as the test this was a variant of. |
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
addisonj
added a commit
to streamnative/pulsar-archived
that referenced
this pull request
Mar 1, 2021
* Fix marking individual deletes as dirty When we mark cursors as dirty, we aren't marking when individual acks cause a dirty cursor. This results in cursors not being flushed and causing redelivery. This one line fix will ensure we mark the cursor as dirty in this situation as well * add a test * improve tests to not use sleep * make the polling rate be slower
zymap
pushed a commit
that referenced
this pull request
Mar 1, 2021
* Fix marking individual deletes as dirty When we mark cursors as dirty, we aren't marking when individual acks cause a dirty cursor. This results in cursors not being flushed and causing redelivery. This one line fix will ensure we mark the cursor as dirty in this situation as well * add a test * improve tests to not use sleep * make the polling rate be slower (cherry picked from commit 34ca893)
zymap
pushed a commit
that referenced
this pull request
Mar 2, 2021
* Fix marking individual deletes as dirty When we mark cursors as dirty, we aren't marking when individual acks cause a dirty cursor. This results in cursors not being flushed and causing redelivery. This one line fix will ensure we mark the cursor as dirty in this situation as well * add a test * improve tests to not use sleep * make the polling rate be slower (cherry picked from commit 34ca893)
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.
When we mark cursors as dirty, we aren't marking when individual acks
cause a dirty cursor.
This results in cursors not being flushed and causing redelivery.
This one line fix will ensure we mark the cursor as dirty in this
situation as well