You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/07/25 08:49:09 UTC
[GitHub] [bookkeeper] wenbingshen opened a new pull request, #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
wenbingshen opened a new pull request, #3423:
URL: https://github.com/apache/bookkeeper/pull/3423
### Motivation
1.`waitForReadOnlyBookie` adds another `listener` thread to observe the node status of bookie, which may be out of sync with the triggering of node changes in `EnsemblePlacementPolicy`.This `sequence` leads to flaky test.
2.Expose bookie nodes in EnsemblePlacementPolicy to client.
3.Just change from watching zk to thread sleep.
We currently don't have any additional requirements for getting bookie nodes on the client side, so it's better to just use thread sleep here.
### Changes
Change from `waitForReadOnlyBookie` and `waitForWritableBookie` to thread `sleep`.
3 seconds is enough for most.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen closed pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
wenbingshen closed pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
URL: https://github.com/apache/bookkeeper/pull/3423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on a diff in pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on code in PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423#discussion_r929536256
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ReadOnlyBookieTest.java:
##########
@@ -128,8 +129,10 @@ public void testBookieShouldTurnWritableFromReadOnly() throws Exception {
// Expected
}
- bkc.waitForReadOnlyBookie(BookieImpl.getBookieId(confByIndex(1)))
- .get(30, TimeUnit.SECONDS);
+ // waitForReadOnlyBookie adds another listener thread to observe the node status of bookie,
+ // which may be out of sync with the triggering of node changes in EnsemblePlacementPolicy.
+ // This sequence leads to flaky test. So change from watching zk to thread sleep.
+ Thread.sleep(3000);
Review Comment:
sure. I have addressed your comments. PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423#issuecomment-1193768856
ping @hangc0276 @Shoothzj @dlg99 @eolivelli @nicoloboschi PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423#issuecomment-1197629953
reopen trigger CI again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423#issuecomment-1193845760
rerun failure checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 merged pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
hangc0276 merged PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on code in PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423#discussion_r929416509
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ReadOnlyBookieTest.java:
##########
@@ -128,8 +129,10 @@ public void testBookieShouldTurnWritableFromReadOnly() throws Exception {
// Expected
}
- bkc.waitForReadOnlyBookie(BookieImpl.getBookieId(confByIndex(1)))
- .get(30, TimeUnit.SECONDS);
+ // waitForReadOnlyBookie adds another listener thread to observe the node status of bookie,
+ // which may be out of sync with the triggering of node changes in EnsemblePlacementPolicy.
+ // This sequence leads to flaky test. So change from watching zk to thread sleep.
+ Thread.sleep(3000);
Review Comment:
Would you please use `Awayatility` instead of sleep ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on pull request #3423: fix Flaky-test: testBookieShouldTurnWritableFromReadOnly
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on PR #3423:
URL: https://github.com/apache/bookkeeper/pull/3423#issuecomment-1198123785
rerun failure checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org