-
Notifications
You must be signed in to change notification settings - Fork 964
Delete cookie as part of decommission API #4592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I suggest temporarily closing this PR and reopening it later if necessary. |
|
The command line decommission will delete cookies by default, but the http api does not have the alignment function |
StevenLuMT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good jobs:
The command line decommission will delete cookies by default, but the http api does not have the alignment function
|
@hezhangjian @zymap help review this pr, thanks |
| Thread.sleep(60 * 1000); | ||
| runFunctionWithRegistrationManager(baseConf, registrationManager -> { | ||
| Versioned<Cookie> cookieFromZk = null; | ||
| try { | ||
| cookieFromZk = Cookie.readFromRegistrationManager(registrationManager, | ||
| bookieId); | ||
| } catch (BookieException e) { | ||
| } finally { | ||
| assertTrue(cookieFromZk != null); | ||
| } | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Awaitility to verify the condition periodically instead of sleeping.
Awaitility.await().untilAsserted(() -> {
assertTrue("Bookie should be running and converted to readonly mode",
bookie.isRunning() && bookie.isReadOnly());
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use Awaitility to verify the condition periodically instead of sleeping.
Awaitility.await().untilAsserted(() -> { assertTrue("Bookie should be running and converted to readonly mode", bookie.isRunning() && bookie.isReadOnly()); });
fixed, Thanks for your suggestion.
| HttpServiceResponse response1 = decommissionService.handle(request1); | ||
| assertEquals(HttpServer.StatusCode.OK.getValue(), response1.getStatusCode()); | ||
| // wait decommission finish | ||
| Thread.sleep(60 * 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same above.
fixed, Thanks for your suggestion.
4798852 to
eb8be0a
Compare
AbdulElahOthmanGwaith
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// wait decommission finish
Awaitility.await()
.untilAsserted(() -> {
Versioned clusterInfo =
admin.getClusterInfo();
assertFalse(
clusterInfo.getValue()
.getBookies()
.contains(bookieId)
);
});
Implement the logic in BP-68: Delete cookie as part of decommission API.