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 2020/10/23 11:36:29 UTC
[GitHub] [bookkeeper] eolivelli opened a new pull request #2453: BP-41 Cleanup tools logs
eolivelli opened a new pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453
### Motivation
BP-41 introduces the new bookieId concept, and tools have to be improvement in order to provide better user experience.
### Changes
- Fix NPE while shutting down the ZKRegistration client no function impact)
- Clean up logs in general
- Print "bookieId" instead of Bookie Address in "bin/bkctl bookies list" command
- Print bookieId at Bookie startup (using standard scripts)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #2453: BP-41 Cleanup tools logs
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453#issuecomment-716212111
@Ghatage 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Ghatage merged pull request #2453: BP-41 Cleanup tools logs
Posted by GitBox <gi...@apache.org>.
Ghatage merged pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2453: BP-41 Cleanup tools logs
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453#discussion_r510822320
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
##########
@@ -462,6 +465,9 @@ public synchronized void unwatchReadOnlyBookies(RegistrationListener listener) {
}
private static BookieId stripBookieIdFromPath(String path) {
+ if (path == null) {
Review comment:
this path can be null for system ZK events, like ConnectionLoss
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Ghatage commented on a change in pull request #2453: BP-41 Cleanup tools logs
Posted by GitBox <gi...@apache.org>.
Ghatage commented on a change in pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453#discussion_r512407854
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
##########
@@ -254,23 +254,14 @@ private static ServerConfiguration parseCommandLine(String[] args)
printUsage();
throw iae;
}
-
- StringBuilder sb = new StringBuilder();
- String[] ledgerDirNames = conf.getLedgerDirNames();
- for (int i = 0; i < ledgerDirNames.length; i++) {
- if (i != 0) {
- sb.append(',');
- }
- sb.append(ledgerDirNames[i]);
- }
-
String hello = String.format(
- "Hello, I'm your bookie, listening on port %1$s. Metadata service uri is %2$s."
- + " Journals are in %3$s. Ledgers are stored in %4$s.",
+ "Hello, I'm your bookie, bookieId is %1$s, listening on port %2$s. Metadata service uri is %3$s."
+ + " Journals are in %4$s. Ledgers are stored in %5$s.",
+ conf.getBookieId() != null ? conf.getBookieId() : "<not-set>",
conf.getBookiePort(),
conf.getMetadataServiceUriUnchecked(),
Arrays.asList(conf.getJournalDirNames()),
- sb);
+ Arrays.asList(conf.getLedgerDirNames()));
Review comment:
Good job on the clean up here. We should have started to use it this way a long time ago.
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/helpers/CommandHelpers.java
##########
@@ -59,14 +58,14 @@ public static String getBookieSocketAddrStringRepresentation(BookieId bookidId,
}
realHostname = hostname;
}
- return formatBookieSocketAddress(bookieID, ip, networkAddress.getPort(), realHostname);
+ return formatBookieSocketAddress(bookidId, ip, networkAddress.getPort(), realHostname);
Review comment:
nit: typo `bookidId` should be `bookieId`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2453: BP-41 Cleanup tools logs
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453#discussion_r512534910
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/helpers/CommandHelpers.java
##########
@@ -59,14 +58,14 @@ public static String getBookieSocketAddrStringRepresentation(BookieId bookidId,
}
realHostname = hostname;
}
- return formatBookieSocketAddress(bookieID, ip, networkAddress.getPort(), realHostname);
+ return formatBookieSocketAddress(bookidId, ip, networkAddress.getPort(), realHostname);
Review comment:
fixed. @Ghatage PTAL 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Ghatage commented on pull request #2453: BP-41 Cleanup tools logs
Posted by GitBox <gi...@apache.org>.
Ghatage commented on pull request #2453:
URL: https://github.com/apache/bookkeeper/pull/2453#issuecomment-716334872
Will take a look tomorrow.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org