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/02/16 10:08:07 UTC
[GitHub] [bookkeeper] wuYin opened a new pull request #3055: check all bookies of writeset are writable
wuYin opened a new pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055
### Motivation
#1088 introduced ensemble writable checking before sending requests, but we should check bookies of writeset, instead of the first few bookies in current ensemble.
### Changes
Get the bookies of writeset from ensemble and check writeable.
Related change: https://github.com/apache/bookkeeper/pull/1088/files#diff-1d893bb31553b5e1f55c8301d04ae15f38e0d35f531f9dd22475128b7972ddf9R1108
--
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] dlg99 commented on pull request #3055: check all bookies of writeset are writable
Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055#issuecomment-1051387813
@wuYin are you working on the tests? I'd love to include this into release 4.15 with appropriate tests
--
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] dlg99 commented on a change in pull request #3055: check all bookies of writeset are writable
Posted by GitBox <gi...@apache.org>.
dlg99 commented on a change in pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055#discussion_r808264614
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
##########
@@ -1224,7 +1224,9 @@ private boolean isWriteSetWritable(DistributionSchedule.WriteSet writeSet,
int nonWritableCount = 0;
List<BookieId> currentEnsemble = getCurrentEnsemble();
for (int i = 0; i < sz; i++) {
- if (!clientCtx.getBookieClient().isWritable(currentEnsemble.get(i), ledgerId)) {
+ int writeBookieIndex = writeSet.get(i);
+ if (writeBookieIndex < currentEnsemble.size() &&
+ !clientCtx.getBookieClient().isWritable(currentEnsemble.get(writeBookieIndex), key)) {
Review comment:
```suggestion
!clientCtx.getBookieClient().isWritable(currentEnsemble.get(writeBookieIndex), ledgerId)) {
```
--
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] dlg99 commented on pull request #3055: check all bookies of writeset are writable
Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055#issuecomment-1041905162
@wuYin please fix compile error and consider adding unit test
```
/home/runner/work/bookkeeper/bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java:1229: error: cannot find symbol
!clientCtx.getBookieClient().isWritable(currentEnsemble.get(writeBookieIndex), key)) {
^
symbol: variable key
location: class LedgerHandle
> Task :bookkeeper-server:compileJava
1 error
> Task :bookkeeper-server:compileJava FAILED
```
--
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] dlg99 merged pull request #3055: check all bookies of writeset are writable
Posted by GitBox <gi...@apache.org>.
dlg99 merged pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055
--
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] dlg99 commented on pull request #3055: check all bookies of writeset are writable
Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055#issuecomment-1043560077
@wuYin can you please take care of checkstyle errors and add/extend the tests?
Take a look at SlowBookieTest and BookieBackpressureTest as starting points.
I hope we can get this into 4.15 release
```
> Task :bookkeeper-server:checkstyleMain
Error: eckstyle] [ERROR] /home/runner/work/bookkeeper/bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java:1228:59: '&&' should be on a new line.
```
--
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] wuYin commented on pull request #3055: check all bookies of writeset are writable
Posted by GitBox <gi...@apache.org>.
wuYin commented on pull request #3055:
URL: https://github.com/apache/bookkeeper/pull/3055#issuecomment-1052074344
@dlg99
Sorry for the late reply, now I added writeset writeable check, this test covered the bug, but not involved back-pressure.
Thank you for your patience.
--
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