Copilot commented on code in PR #4646:
URL: https://github.com/apache/bookkeeper/pull/4646#discussion_r2264980478
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java:
##########
@@ -358,7 +430,24 @@ private CompletableFuture<Versioned<Set<BookieId>>>
getChildren(String regPath,
List<CompletableFuture<Versioned<BookieServiceInfo>>>
bookieInfoUpdated = new ArrayList<>(bookies.size());
for (BookieId id : bookies) {
// update the cache for new bookies
- if (!bookieServiceInfoCache.containsKey(id)) {
+ if (path.equals(bookieReadonlyRegistrationPath)) {
+ if (readOnlyBookieInfo.get(id) == null) {
+
bookieInfoUpdated.add(readBookieInfoAsReadonlyBookie(id));
+ continue;
+ }
+ }
+ if (path.equals(bookieRegistrationPath)) {
+ if (writableBookieInfo.get(id) == null) {
+
bookieInfoUpdated.add(readBookieInfoAsReadonlyBookie(id));
Review Comment:
This line should call `readBookieInfoAsWritableBookie(id)` instead of
`readBookieInfoAsReadonlyBookie(id)` since it's in the branch handling writable
bookies (`bookieRegistrationPath`).
```suggestion
bookieInfoUpdated.add(readBookieInfoAsWritableBookie(id));
```
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java:
##########
@@ -263,31 +271,72 @@ private CompletableFuture<Versioned<BookieServiceInfo>>
readBookieServiceInfoAsy
CompletableFuture<Versioned<BookieServiceInfo>> promise = new
CompletableFuture<>();
zk.getData(pathAsWritable, bookieServiceInfoCacheInvalidation,
(int rc, String path, Object o, byte[] bytes, Stat stat) -> {
- if (KeeperException.Code.OK.intValue() == rc) {
- try {
- BookieServiceInfo bookieServiceInfo =
deserializeBookieServiceInfo(bookieId, bytes);
- Versioned<BookieServiceInfo> result = new
Versioned<>(bookieServiceInfo,
- new LongVersion(stat.getCversion()));
- log.info("Update BookieInfoCache (writable bookie) {} ->
{}", bookieId, result.getValue());
- bookieServiceInfoCache.put(bookieId, result);
- promise.complete(result);
- } catch (IOException ex) {
- log.error("Cannot update BookieInfo for ", ex);
-
promise.completeExceptionally(KeeperException.create(KeeperException.Code.get(rc),
path)
- .initCause(ex));
- return;
- }
- } else if (KeeperException.Code.NONODE.intValue() == rc) {
- // not found, looking for a readonly bookie
- zk.getData(pathAsReadonly, bookieServiceInfoCacheInvalidation,
- (int rc2, String path2, Object o2, byte[] bytes2, Stat
stat2) -> {
+ if (KeeperException.Code.OK.intValue() == rc) {
+ try {
+ BookieServiceInfo bookieServiceInfo =
deserializeBookieServiceInfo(bookieId, bytes);
+ Versioned<BookieServiceInfo> result = new
Versioned<>(bookieServiceInfo,
+ new LongVersion(stat.getCversion()));
+ log.info("Update BookieInfoCache (writable bookie)
{} -> {}", bookieId, result.getValue());
+ writableBookieInfo.put(bookieId, result);
+ promise.complete(result);
+ } catch (IOException ex) {
+ log.error("Cannot update BookieInfo for ", ex);
+
promise.completeExceptionally(KeeperException.create(KeeperException.Code.get(rc),
path)
+ .initCause(ex));
+ return;
+ }
+ } else if (KeeperException.Code.NONODE.intValue() == rc) {
+ // not found, looking for a readonly bookie
+ zk.getData(pathAsReadonly,
bookieServiceInfoCacheInvalidation,
+ (int rc2, String path2, Object o2, byte[]
bytes2, Stat stat2) -> {
+ if (KeeperException.Code.OK.intValue() ==
rc2) {
+ try {
+ BookieServiceInfo
bookieServiceInfo =
+
deserializeBookieServiceInfo(bookieId, bytes2);
+ Versioned<BookieServiceInfo>
result =
+ new
Versioned<>(bookieServiceInfo,
+ new
LongVersion(stat2.getCversion()));
+ log.info("Update BookieInfoCache
(readonly bookie) {} -> {}",
+ bookieId,
result.getValue());
+ readOnlyBookieInfo.put(bookieId,
result);
+ promise.complete(result);
+ } catch (IOException ex) {
+ log.error("Cannot update
BookieInfo for ", ex);
+ promise.completeExceptionally(
+
KeeperException.create(KeeperException.Code.get(rc2), path2)
+ .initCause(ex)
+ );
+ return;
+ }
+ } else {
+ // not found as writable and readonly,
the bookie is offline
+ promise.completeExceptionally(
+
BKException.create(BKException.Code.NoBookieAvailableException)
+ );
+ }
+ }, null);
+ } else {
+
promise.completeExceptionally(KeeperException.create(KeeperException.Code.get(rc),
path));
+ }
+ }, null);
+ return promise;
+ }
+
+
+ private CompletableFuture<Versioned<BookieServiceInfo>>
readBookieInfoAsReadonlyBookie(BookieId bookieId) {
Review Comment:
This method duplicates logic already present in the main
`readBookieServiceInfoAsync` method. Consider removing this duplicate method
and using the existing logic.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java:
##########
@@ -300,10 +349,33 @@ private CompletableFuture<Versioned<BookieServiceInfo>>
readBookieServiceInfoAsy
promise.completeExceptionally(BKException.create(BKException.Code.NoBookieAvailableException));
}
}, null);
- } else {
-
promise.completeExceptionally(KeeperException.create(KeeperException.Code.get(rc),
path));
- }
- }, null);
+ return promise;
+ }
+
+ private CompletableFuture<Versioned<BookieServiceInfo>>
readBookieInfoAsWritableBookie(BookieId bookieId) {
Review Comment:
This method duplicates logic already present in the main
`readBookieServiceInfoAsync` method. Consider removing this duplicate method
and using the existing logic.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
