[GitHub] jvrao commented on a change in pull request #1121: [Merge Yahoo repo]: AsyncReadLastEntry should trigger callback with error when ledger is empty

[GitBox]

jvrao commented on a change in pull request #1121: [Merge Yahoo repo]: 
AsyncReadLastEntry should trigger callback with error when ledger is empty
URL: https://github.com/apache/bookkeeper/pull/1121#discussion_r168089437
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
 ##
 @@ -787,6 +787,24 @@ public void onFailure(Throwable cause) {
 }
 }
 
+/*
+ * Read the last entry in the ledger
+ *
+ * @param cb
+ *object implementing read callback interface
+ * @param ctx
+ *control object
+ */
+public void asyncReadLastEntry(ReadCallback cb, Object ctx) {
+long lastEntryId = getLastAddConfirmed();
 
 Review comment:
   +1 on Sijie's comment


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jiazhai commented on issue #1121: [Merge Yahoo repo]: AsyncReadLastEntry should trigger callback with error when ledger is empty

[GitBox]

jiazhai commented on issue #1121: [Merge Yahoo repo]: AsyncReadLastEntry should 
trigger callback with error when ledger is empty
URL: https://github.com/apache/bookkeeper/pull/1121#issuecomment-365487833
 
 
   @sijie Thanks for the comments. changed the code. CI failure seems caused by 
other un-related issue.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kishorvpatil commented on issue #1125: Issue #1124: Lower memory usage in GarbageCollectionThread while extracting all ledger meta data

[GitBox]

kishorvpatil commented on issue #1125: Issue #1124: Lower memory usage in 
GarbageCollectionThread while extracting all ledger meta data
URL: https://github.com/apache/bookkeeper/pull/1125#issuecomment-365485471
 
 
   @eolivelli  Sorry for delay on this. i have fixed the checkstyle issue and 
also fixed other test comments.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kishorvpatil commented on issue #1125: Issue #1124: Lower memory usage in GarbageCollectionThread while extracting all ledger meta data

[GitBox]

kishorvpatil commented on issue #1125: Issue #1124: Lower memory usage in 
GarbageCollectionThread while extracting all ledger meta data
URL: https://github.com/apache/bookkeeper/pull/1125#issuecomment-365485584
 
 
   @sijie Removing teardown requires ensure we run this particular test always 
with throttle enabled.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1121: [Merge Yahoo repo]: AsyncReadLastEntry should trigger callback with error when ledger is empty

[GitBox]

sijie commented on a change in pull request #1121: [Merge Yahoo repo]: 
AsyncReadLastEntry should trigger callback with error when ledger is empty
URL: https://github.com/apache/bookkeeper/pull/1121#discussion_r167386207
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestReadLastEntryAsync.java
 ##
 @@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.bookkeeper.client;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Enumeration;
+import java.util.concurrent.CountDownLatch;
+import org.apache.bookkeeper.client.AsyncCallback.ReadCallback;
+import org.apache.bookkeeper.client.BookKeeper.DigestType;
+import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.junit.Test;
+
+/**
+  * Test read next entry and the latest last add confirmed.
+  */
+public class TestReadLastEntryAsync extends BookKeeperClusterTestCase {
+
+final DigestType digestType;
+
+public TestReadLastEntryAsync() {
+super(3);
+this.digestType = DigestType.CRC32;
+}
+
+@Test(timeout = 6)
 
 Review comment:
   drop the `timeout` here?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1121: [Merge Yahoo repo]: AsyncReadLastEntry should trigger callback with error when ledger is empty

[GitBox]

sijie commented on a change in pull request #1121: [Merge Yahoo repo]: 
AsyncReadLastEntry should trigger callback with error when ledger is empty
URL: https://github.com/apache/bookkeeper/pull/1121#discussion_r167386849
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
 ##
 @@ -787,6 +787,24 @@ public void onFailure(Throwable cause) {
 }
 }
 
+/*
+ * Read the last entry in the ledger
+ *
+ * @param cb
+ *object implementing read callback interface
+ * @param ctx
+ *control object
+ */
+public void asyncReadLastEntry(ReadCallback cb, Object ctx) {
 
 Review comment:
   Let's add a sync version for this method as well. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1121: [Merge Yahoo repo]: AsyncReadLastEntry should trigger callback with error when ledger is empty

[GitBox]

sijie commented on a change in pull request #1121: [Merge Yahoo repo]: 
AsyncReadLastEntry should trigger callback with error when ledger is empty
URL: https://github.com/apache/bookkeeper/pull/1121#discussion_r167386297
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestReadLastEntryAsync.java
 ##
 @@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.bookkeeper.client;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Enumeration;
+import java.util.concurrent.CountDownLatch;
+import org.apache.bookkeeper.client.AsyncCallback.ReadCallback;
+import org.apache.bookkeeper.client.BookKeeper.DigestType;
+import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.junit.Test;
+
+/**
+  * Test read next entry and the latest last add confirmed.
+  */
+public class TestReadLastEntryAsync extends BookKeeperClusterTestCase {
+
+final DigestType digestType;
+
+public TestReadLastEntryAsync() {
+super(3);
 
 Review comment:
   I would reduce this to 1. because we don't actually need 3 bookies for 
testing this feature.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1121: [Merge Yahoo repo]: AsyncReadLastEntry should trigger callback with error when ledger is empty

[GitBox]

sijie commented on a change in pull request #1121: [Merge Yahoo repo]: 
AsyncReadLastEntry should trigger callback with error when ledger is empty
URL: https://github.com/apache/bookkeeper/pull/1121#discussion_r167386280
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestReadLastEntryAsync.java
 ##
 @@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.bookkeeper.client;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Enumeration;
+import java.util.concurrent.CountDownLatch;
+import org.apache.bookkeeper.client.AsyncCallback.ReadCallback;
+import org.apache.bookkeeper.client.BookKeeper.DigestType;
+import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.junit.Test;
+
+/**
+  * Test read next entry and the latest last add confirmed.
+  */
+public class TestReadLastEntryAsync extends BookKeeperClusterTestCase {
+
+final DigestType digestType;
+
+public TestReadLastEntryAsync() {
+super(3);
+this.digestType = DigestType.CRC32;
+}
+
+@Test(timeout = 6)
+public void testTryReadLastEntryAsyncOnEmptyLedger() throws Exception {
+final LedgerHandle lh = bkc.createLedger(3, 3, 1, digestType, 
"".getBytes());
+lh.close();
+
+LedgerHandle readLh = bkc.openLedger(lh.getId(), digestType, 
"".getBytes());
+
+final CountDownLatch latch1 = new CountDownLatch(1);
+readLh.asyncReadLastEntry(new ReadCallback() {
+@Override
+public void readComplete(int rc, LedgerHandle lh, 
Enumeration seq, Object ctx) {
+assertEquals(BKException.Code.NoSuchEntryException, rc);
 
 Review comment:
   I don't think we should `assert` directly in a callback. because the 
callback will be executed in a worker scheduler thread.
   
   You can have a `AtomicInteger` or `MutableInteger` to hold the result `rc`, 
and check the result after `latch1.await()`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jvrao commented on a change in pull request #1140: Issue 1139: Add debug to replication fencing

[GitBox]

jvrao commented on a change in pull request #1140: Issue 1139: Add debug to 
replication fencing
URL: https://github.com/apache/bookkeeper/pull/1140#discussion_r167447579
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java
 ##
 @@ -364,40 +368,52 @@ private void deferLedgerLockRelease(final long ledgerId) 
{
 TimerTask timerTask = new TimerTask() {
 @Override
 public void run() {
+boolean isRecoveryOpen = false;
 LedgerHandle lh = null;
 try {
 lh = admin.openLedgerNoRecovery(ledgerId);
 if (isLastSegmentOpenAndMissingBookies(lh)) {
+// Need recovery open, close the old ledger handle.
+lh.close();
+// Recovery open could result in client write failure.
+LOG.warn("Missing bookie(s) from last segment. Opening 
Ledger{} for Recovery.", ledgerId);
 lh = admin.openLedger(ledgerId);
+isRecoveryOpen = true;
 }
-
-Set fragments = 
getUnderreplicatedFragments(lh);
-for (LedgerFragment fragment : fragments) {
-if (!fragment.isClosed()) {
-lh = admin.openLedger(ledgerId);
-break;
+if (!isRecoveryOpen){
+Set fragments = 
getUnderreplicatedFragments(lh);
+for (LedgerFragment fragment : fragments) {
+if (!fragment.isClosed()) {
+// Need recovery open, close the old ledger 
handle.
+lh.close();
+// Recovery open could result in client write 
failure.
+LOG.warn("Open Fragment{}. Opening Ledger{} 
for Recovery.",
+fragment.getEnsemble(), ledgerId);
+lh = admin.openLedger(ledgerId);
+isRecoveryOpen = true;
+break;
+}
 }
 }
 } catch (InterruptedException e) {
 Thread.currentThread().interrupt();
-LOG.info("InterruptedException "
-+ "while replicating fragments", e);
+LOG.info("InterruptedException while fencing the ledger {}"
++ " for rereplication of postponed ledgers", 
lh.getId(), e);
 } catch (BKNoSuchLedgerExistsException bknsle) {
 if (LOG.isDebugEnabled()) {
 LOG.debug("Ledger was deleted, safe to continue", 
bknsle);
 }
 } catch (BKException e) {
-LOG.error("BKException while fencing the ledger"
-+ " for rereplication of postponed ledgers", e);
+LOG.error("BKException while fencing the ledger {}"
++ " for rereplication of postponed ledgers", 
lh.getId(), e);
 } finally {
 try {
 if (lh != null) {
 lh.close();
 }
 } catch (InterruptedException e) {
 Thread.currentThread().interrupt();
-LOG.info("InterruptedException while closing "
-+ "ledger", e);
+LOG.info("InterruptedException while closing ledger", 
e);
 
 Review comment:
   @merlimat  I will add ledgerId to every log message in this method.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jvrao commented on a change in pull request #1140: Issue 1139: Add debug to replication fencing

[GitBox]

jvrao commented on a change in pull request #1140: Issue 1139: Add debug to 
replication fencing
URL: https://github.com/apache/bookkeeper/pull/1140#discussion_r167934280
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java
 ##
 @@ -364,40 +368,52 @@ private void deferLedgerLockRelease(final long ledgerId) 
{
 TimerTask timerTask = new TimerTask() {
 @Override
 public void run() {
+boolean isRecoveryOpen = false;
 LedgerHandle lh = null;
 try {
 lh = admin.openLedgerNoRecovery(ledgerId);
 if (isLastSegmentOpenAndMissingBookies(lh)) {
+// Need recovery open, close the old ledger handle.
+lh.close();
+// Recovery open could result in client write failure.
+LOG.warn("Missing bookie(s) from last segment. Opening 
Ledger{} for Recovery.", ledgerId);
 lh = admin.openLedger(ledgerId);
+isRecoveryOpen = true;
 }
-
-Set fragments = 
getUnderreplicatedFragments(lh);
-for (LedgerFragment fragment : fragments) {
-if (!fragment.isClosed()) {
-lh = admin.openLedger(ledgerId);
-break;
+if (!isRecoveryOpen){
 
 Review comment:
   @sijie  All we are doing in this block is to open the ledger in recovery 
mode isn't it?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly closed pull request #1132: Fix shutdown race which left ZK session open

[GitBox]

ivankelly closed pull request #1132: Fix shutdown race which left ZK session 
open
URL: https://github.com/apache/bookkeeper/pull/1132
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
index c2f7b8e5c..5ee8bb549 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
@@ -219,7 +219,6 @@ static int doMain(String[] args) {
 // the server is interrupted
 log.info("Bookie server is interrupted. Exiting ...");
 }
-server.close();
 return ExitCode.OK;
 }
 


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly commented on issue #1132: Fix shutdown race which left ZK session open

[GitBox]

ivankelly commented on issue #1132: Fix shutdown race which left ZK session open
URL: https://github.com/apache/bookkeeper/pull/1132#issuecomment-365328837
 
 
   Merging
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly closed pull request #1145: Require green CI before merge

[GitBox]

ivankelly closed pull request #1145: Require green CI before merge
URL: https://github.com/apache/bookkeeper/pull/1145
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/dev/bk-merge-pr.py b/dev/bk-merge-pr.py
index 8c3415e20..c5dffe713 100755
--- a/dev/bk-merge-pr.py
+++ b/dev/bk-merge-pr.py
@@ -476,6 +476,19 @@ def get_reviewers(pr_num):
 reviewers_emails.append('{0} <{1}>'.format(username.encode('utf8'), 
useremail))
 return ', '.join(reviewers_emails)
 
+def check_ci_status(pr):
+status_url = get_json("%s/commits/%s/status" % (GITHUB_API_BASE, 
pr["head"]["sha"]))
+state = status_url["state"]
+if state != "success":
+comments = get_json(pr["comments_url"])
+ignore_ci_comments = [c for c in comments if c["body"].upper() == 
"IGNORE CI"]
+if len(ignore_ci_comments) > 0:
+print "\n\nWARNING: The PR has not passed CI (state is %s)" % 
(state) \
++ ", but this has been overridden by %s. \n" % 
(ignore_ci_comments[0]["user"]["login"]) \
++ "Proceed at your own peril!\n\n"
+else:
+fail("The PR has not passed CI (state is %s)" % (state))
+
 def ask_release_for_github_issues(branch, labels):
 print "=== Add release to github issues ==="
 while True:
@@ -643,9 +656,11 @@ def main():
 pr = get_json("%s/pulls/%s" % (GITHUB_API_BASE, pr_num))
 pr_events = get_json("%s/issues/%s/events" % (GITHUB_API_BASE, pr_num))
 pr_reviewers = get_reviewers(pr_num)
+check_ci_status(pr)
+
 url = pr["url"]
 
-# 3. repare the title for commit message
+# 3. repair the title for commit message
 pr_title = pr["title"]
 commit_title = raw_input("Commit title [%s]: " % 
pr_title.encode("utf-8")).decode("utf-8")
 if commit_title == "":


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly commented on issue #1145: Require green CI before merge

[GitBox]

ivankelly commented on issue #1145: Require green CI before merge
URL: https://github.com/apache/bookkeeper/pull/1145#issuecomment-365328267
 
 
   Merging


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Jenkins build is unstable: bookkeeper_postcommit_master_java9 #41

[Apache Jenkins Server]

See 

Jenkins build is unstable: bookkeeper_release_nightly_snapshot #58

[Apache Jenkins Server]

See 

[GitHub] sijie commented on a change in pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

sijie commented on a change in pull request #1137: Fix test regression due to 
change in default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#discussion_r167886682
 
 

 ##
 File path: 
bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 ##
 @@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
 
 Review comment:
   yes, I am not blocking this PR. the comment was just "FYI"


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli commented on issue #1125: Issue #1124: Lower memory usage in GarbageCollectionThread while extracting all ledger meta data

[GitBox]

eolivelli commented on issue #1125: Issue #1124: Lower memory usage in 
GarbageCollectionThread while extracting all ledger meta data
URL: https://github.com/apache/bookkeeper/pull/1125#issuecomment-365287901
 
 
   @kishorvpatil checkstyle is not passing, please check


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli commented on a change in pull request #1125: Issue #1124: Lower memory usage in GarbageCollectionThread while extracting all ledger meta data

[GitBox]

eolivelli commented on a change in pull request #1125: Issue #1124: Lower 
memory usage in GarbageCollectionThread while extracting all ledger meta data
URL: https://github.com/apache/bookkeeper/pull/1125#discussion_r167884567
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
 ##
 @@ -835,6 +837,69 @@ public void checkpointComplete(Checkpoint checkpoint,
 storage.gcThread.doCompactEntryLogs(threshold);
 }
 
+/**
+ * Test extractMetaFromEntryLogs optimized method to avoid excess memory 
usage.
+ */
+@Test(timeout = 6)
+public void testExtractMetaFromEntryLogs() throws Exception {
+ServerConfiguration conf = 
TestBKConfiguration.newServerConfiguration();
+File tmpDir = createTempDir("bkTest", ".dir");
+File curDir = Bookie.getCurrentDirectory(tmpDir);
+Bookie.checkDirectoryStructure(curDir);
+conf.setLedgerDirNames(new String[] { tmpDir.toString() });
+
+LedgerDirsManager dirs = new LedgerDirsManager(conf, 
conf.getLedgerDirs(),
+new DiskChecker(conf.getDiskUsageThreshold(), 
conf.getDiskUsageWarnThreshold()));
+final Set ledgers = Collections
+.newSetFromMap(new ConcurrentHashMap());
+
+LedgerManager manager = getLedgerManager(ledgers);
+
+CheckpointSource checkpointSource = new CheckpointSource() {
+
+@Override
+public Checkpoint newCheckpoint() {
+return null;
+}
+
+@Override
+public void checkpointComplete(Checkpoint checkpoint,
+   boolean compact) throws IOException 
{
+}
+};
+InterleavedLedgerStorage storage = new InterleavedLedgerStorage();
+storage.initialize(conf, manager, dirs, dirs, null, checkpointSource, 
Checkpointer.NULL, NullStatsLogger.INSTANCE);
+final byte[] KEY = "foobar".getBytes();
+
+for (long ledger = 0; ledger <= 10; ledger++) {
+ledgers.add(ledger);
+for(int entry = 1; entry <= 50; entry++) {
+try {
+storage.addEntry(genEntry(ledger, entry, ENTRY_SIZE));
+} catch (IOException e) {
+//ignore exception on failure to add entry.
+}
+}
+}
+
+storage.flush();
+storage.shutdown();
+
+storage = new InterleavedLedgerStorage();
 
 Review comment:
   @sijie fine, I suspected it. fine for me


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1125: Issue #1124: Lower memory usage in GarbageCollectionThread while extracting all ledger meta data

[GitBox]

sijie commented on a change in pull request #1125: Issue #1124: Lower memory 
usage in GarbageCollectionThread while extracting all ledger meta data
URL: https://github.com/apache/bookkeeper/pull/1125#discussion_r167884231
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
 ##
 @@ -835,6 +837,69 @@ public void checkpointComplete(Checkpoint checkpoint,
 storage.gcThread.doCompactEntryLogs(threshold);
 }
 
+/**
+ * Test extractMetaFromEntryLogs optimized method to avoid excess memory 
usage.
+ */
+@Test(timeout = 6)
+public void testExtractMetaFromEntryLogs() throws Exception {
+ServerConfiguration conf = 
TestBKConfiguration.newServerConfiguration();
+File tmpDir = createTempDir("bkTest", ".dir");
+File curDir = Bookie.getCurrentDirectory(tmpDir);
+Bookie.checkDirectoryStructure(curDir);
+conf.setLedgerDirNames(new String[] { tmpDir.toString() });
+
+LedgerDirsManager dirs = new LedgerDirsManager(conf, 
conf.getLedgerDirs(),
+new DiskChecker(conf.getDiskUsageThreshold(), 
conf.getDiskUsageWarnThreshold()));
+final Set ledgers = Collections
+.newSetFromMap(new ConcurrentHashMap());
+
+LedgerManager manager = getLedgerManager(ledgers);
+
+CheckpointSource checkpointSource = new CheckpointSource() {
+
+@Override
+public Checkpoint newCheckpoint() {
+return null;
+}
+
+@Override
+public void checkpointComplete(Checkpoint checkpoint,
+   boolean compact) throws IOException 
{
+}
+};
+InterleavedLedgerStorage storage = new InterleavedLedgerStorage();
+storage.initialize(conf, manager, dirs, dirs, null, checkpointSource, 
Checkpointer.NULL, NullStatsLogger.INSTANCE);
+final byte[] KEY = "foobar".getBytes();
+
+for (long ledger = 0; ledger <= 10; ledger++) {
+ledgers.add(ledger);
+for(int entry = 1; entry <= 50; entry++) {
+try {
+storage.addEntry(genEntry(ledger, entry, ENTRY_SIZE));
+} catch (IOException e) {
+//ignore exception on failure to add entry.
+}
+}
+}
+
+storage.flush();
+storage.shutdown();
+
+storage = new InterleavedLedgerStorage();
 
 Review comment:
   @eolivelli the test logic here is more on EntryLogger not ledger storage. We 
don't need to run same test for two different ledger storage.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Jenkins build is unstable: bookkeeper_postcommit_master_java8 #40

[Apache Jenkins Server]

See 

[GitHub] sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI

[GitBox]

sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI 
URL: https://github.com/apache/bookkeeper/pull/1094#discussion_r167881457
 
 

 ##
 File path: 
bookkeeper-tools/src/main/java/org/apache/bookkeeper/tools/cli/commands/CmdBase.java
 ##
 @@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands;
+
+import com.beust.jcommander.JCommander;
+import com.beust.jcommander.Parameter;
+import lombok.AccessLevel;
+import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.tools.cli.helpers.Command;
+
+/**
+ * The command base for other sub commands to extend.
+ */
+@Slf4j
+public abstract class CmdBase {
+
+// Parameters defined for this command
+
+@Parameter(names = { "-h", "--help" }, help = true, hidden = true)
+private boolean help;
+
+// Parameters defined for this command (end)
+
+protected final JCommander commander;
+@Getter(AccessLevel.PUBLIC)
+protected final ServerConfiguration conf;
+
+protected CmdBase(String cmdName, ServerConfiguration conf) {
+this(cmdName, conf, new JCommander());
+}
+
+protected CmdBase(String cmdName, ServerConfiguration conf, JCommander 
commander) {
+this.conf = conf;
+this.commander = commander;
+this.commander.setProgramName("bookie-shell " + cmdName);
 
 Review comment:
   it isn't.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI

[GitBox]

sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI 
URL: https://github.com/apache/bookkeeper/pull/1094#discussion_r167881366
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/helpers/DiscoveryCommand.java
 ##
 @@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.helpers;
+
+import java.util.Optional;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import lombok.Cleanup;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.bookkeeper.client.api.BookKeeper;
+import org.apache.bookkeeper.conf.ClientConfiguration;
+import org.apache.bookkeeper.discover.RegistrationClient;
+import org.apache.bookkeeper.stats.NullStatsLogger;
+import org.apache.bookkeeper.util.ReflectionUtils;
+
+/**
+ * This is a mixin for commands that talks to discovery service.
+ */
+@Slf4j
+public abstract class DiscoveryCommand extends ClientCommand {
+
+@Override
+protected void run(ClientConfiguration conf) throws Exception {
+Class regClientCls = 
conf.getRegistrationClientClass();
+@Cleanup("shutdown") ScheduledExecutorService executor = 
Executors.newSingleThreadScheduledExecutor();
+try (RegistrationClient regClient = 
ReflectionUtils.newInstance(regClientCls)) {
+regClient.initialize(
+conf,
+executor,
+NullStatsLogger.INSTANCE,
+Optional.empty());
+run(regClient);
+}
+}
+
+@Override
+protected void run(BookKeeper bk) throws Exception {
+throw new UnsupportedOperationException("Discovery command only uses 
registration client");
 
 Review comment:
   this is done intentionally. for `discovery command`, we use 
`run(RegistrationClient)` not `run(BookKeeper)`, but we make DiscoveryCommand 
inherits ClientCommand. making it throw unsupported exception is to prevent 
anyone using this branch.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI

[GitBox]

sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI 
URL: https://github.com/apache/bookkeeper/pull/1094#discussion_r167880741
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/cluster/ListBookiesCommand.java
 ##
 @@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.cluster;
+
+import static org.apache.bookkeeper.common.concurrent.FutureUtils.result;
+import static 
org.apache.bookkeeper.tools.cli.helpers.CommandHelpers.getBookieSocketAddrStringRepresentation;
+
+import com.beust.jcommander.Parameter;
+import com.beust.jcommander.Parameters;
+import com.google.common.collect.Lists;
+import java.util.List;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.discover.RegistrationClient;
+import org.apache.bookkeeper.net.BookieSocketAddress;
+import org.apache.bookkeeper.tools.cli.helpers.DiscoveryCommand;
+
+/**
+ * Command to list available bookies.
+ */
+@Accessors(fluent = true)
+@Setter
+@Parameters(commandDescription = "List the bookies, which are running as 
either readwrite or readonly mode.")
+public class ListBookiesCommand extends DiscoveryCommand {
+
+@Parameter(names = { "-rw", "--readwrite" }, description = "Print 
readwrite bookies")
+private boolean readwrite = false;
+@Parameter(names = { "-ro", "--readonly" }, description = "Print readonly 
bookies")
+private boolean readonly = false;
+
+@Override
+protected void run(RegistrationClient regClient) throws Exception {
+List bookies = Lists.newArrayList();
+if (readwrite) {
+bookies.addAll(
+result(
+regClient.getWritableBookies()
+).getValue()
+);
+} else if (readonly) {
+bookies.addAll(
+result(
+regClient.getReadOnlyBookies()
+).getValue()
+);
+}
 
 Review comment:
   nice catch. will add the validation


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI

[GitBox]

sijie commented on a change in pull request #1094: BP-27: New BookKeeper CLI 
URL: https://github.com/apache/bookkeeper/pull/1094#discussion_r167880214
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/cli/commands/client/SimpleTestCommand.java
 ##
 @@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.tools.cli.commands.client;
+
+import static org.apache.bookkeeper.common.concurrent.FutureUtils.result;
+
+import com.beust.jcommander.Parameter;
+import com.beust.jcommander.Parameters;
+import java.util.concurrent.TimeUnit;
+import lombok.Setter;
+import lombok.experimental.Accessors;
+import org.apache.bookkeeper.client.api.BookKeeper;
+import org.apache.bookkeeper.client.api.DigestType;
+import org.apache.bookkeeper.client.api.WriteHandle;
+import org.apache.bookkeeper.tools.cli.helpers.ClientCommand;
+
+/**
+ * A client command that simply tests if a cluster is healthy.
+ */
+@Accessors(fluent = true)
+@Setter
+@Parameters(commandDescription = "Simple test to create a ledger and write 
entries to it.")
+public class SimpleTestCommand extends ClientCommand {
+
+@Parameter(names = { "-e", "--ensemble-size" }, description = "Ensemble 
size (default 3)")
+private int ensembleSize = 3;
+@Parameter(names = { "-w", "--write-quorum-size" }, description = "Write 
quorum size (default 2)")
+private int writeQuorumSize = 2;
+@Parameter(names = { "-a", "--ack-quorum-size" }, description = "Ack 
quorum size (default 2)")
+private int ackQuorumSize = 2;
+@Parameter(names = { "-n", "--num-entries" }, description = "Entries to 
write (default 100)")
+private int numEntries = 100;
 
 Review comment:
   yes. I do this intentionally, because for a simple test command it is okay 
to just test with 100 entries, 1000 seems to too many for me.
   
   but I didn't change the original behavior for old bookie shell, this 
behavior is only changed for new bookie shell. so it doesn't break any BC here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly commented on issue #1142: Gather disk information before CI build

[GitBox]

ivankelly commented on issue #1142: Gather disk information before CI build
URL: https://github.com/apache/bookkeeper/pull/1142#issuecomment-365281898
 
 
   I want to see what the state of the disks is before we start the test. I 
want to see what the disks are in the case of a failure. Is the /tmp separate? 
If so, couldn't we run in the workspace. Absolutely this is for debugging 
purposes, CI is flaking on environmental issues. It needs to be debugged.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on issue #1142: Gather disk information before CI build

[GitBox]

sijie commented on issue #1142: Gather disk information before CI build
URL: https://github.com/apache/bookkeeper/pull/1142#issuecomment-365280640
 
 
   do we really need this change? from the description, it sounds like more for 
debugging purpose. because it is unclear to me what are you going to use these 
information - e.g. what are you going to do if disks are full.
   
   I am asking this is because you don't really need run `df` to know that. 
because I have seen test cases flaky due to NoWritableLedgerDirsException, this 
already tells you the information. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly closed pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

ivankelly closed pull request #1137: Fix test regression due to change in 
default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 
b/bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
index e3b1124f7..205c4ab87 100644
--- 
a/bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
+++ 
b/bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
@@ -25,6 +25,7 @@
 import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
 import org.junit.Test;
 import org.junit.Assert;
+import org.junit.Before;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
+
baseClientConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
+super.setUp();
+}
+
 @Test
 public void testThroughputLatency() throws Exception {
 String latencyFile = System.getProperty("test.latency.file", 
"latencyDump.dat");


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly commented on a change in pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

ivankelly commented on a change in pull request #1137: Fix test regression due 
to change in default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#discussion_r167873701
 
 

 ##
 File path: 
bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 ##
 @@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
 
 Review comment:
   I'm well aware of suppressWarnings, but it was a small nit, that didn't add 
anything, the patch was already green with CI, was otherwise ready to go, just 
waiting on approvals and I wanted to get this change in asap, as master CI has 
been broken for 7 days now, and it's blocking other patches. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly commented on issue #1137: Fix test regression due to change in default ledger manager

[GitBox]

ivankelly commented on issue #1137: Fix test regression due to change in 
default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#issuecomment-365276490
 
 
   merging this now.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

sijie commented on a change in pull request #1137: Fix test regression due to 
change in default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#discussion_r167871390
 
 

 ##
 File path: 
bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 ##
 @@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
 
 Review comment:
   just fyi, you can use full qualified classname and 
`suppresswarnings("deprecation")` on the setup method.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

sijie commented on a change in pull request #1137: Fix test regression due to 
change in default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#discussion_r167871390
 
 

 ##
 File path: 
bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 ##
 @@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
 
 Review comment:
   just fyi, you can use full qualified import and 
`suppresswarnings("deprecation")` on the setup method.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli closed issue #1143: Error on Java 9 using BookKeeperAdmin API and bookkeeper-server-shaded artifact

[GitBox]

eolivelli closed issue #1143: Error on Java 9 using BookKeeperAdmin API and 
bookkeeper-server-shaded artifact
URL: https://github.com/apache/bookkeeper/issues/1143
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli commented on a change in pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

eolivelli commented on a change in pull request #1137: Fix test regression due 
to change in default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#discussion_r167842478
 
 

 ##
 File path: 
bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 ##
 @@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
 
 Review comment:
   Okay


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli closed pull request #1144: Issue-1143 Error on Java 9 using BookKeeperAdmin API and bookkeeper-server-shaded

[GitBox]

eolivelli closed pull request #1144: Issue-1143 Error on Java 9 using 
BookKeeperAdmin API and bookkeeper-server-shaded
URL: https://github.com/apache/bookkeeper/pull/1144
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/pom.xml b/pom.xml
index 0742417d8..bfe43bcc8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -128,7 +128,7 @@
 2.7
 2.2
 2.10.4
-2.4.3
+3.1.0
 2.2.1
 2.19.1
 2.2.1


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ivankelly commented on a change in pull request #1137: Fix test regression due to change in default ledger manager

[GitBox]

ivankelly commented on a change in pull request #1137: Fix test regression due 
to change in default ledger manager
URL: https://github.com/apache/bookkeeper/pull/1137#discussion_r167793795
 
 

 ##
 File path: 
bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java
 ##
 @@ -38,6 +39,13 @@ public TestBenchmark() {
 super(5);
 }
 
+@Before
+public void setUp() throws Exception {
+
baseConf.setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.FlatLedgerManagerFactory");
 
 Review comment:
   Because, if you reference the class,
   ```
   [WARNING] 
/home/ivan/src/bookkeeper/checkouts/bench-flake/bookkeeper-benchmark/src/test/java/org/apache/bookkeeper/benchmark/TestBenchmark.java:[44,73]
 org.apache.bookkeeper.meta.FlatLedgerManagerFactory in 
org.apache.bookkeeper.meta has been deprecated
   ```
   
   FlatLedgerManagerFactory is never going to move. That in itself would be a 
BC break.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services