[GitHub] sijie opened a new pull request #1772: [tests] Fix integration test TestCompatUpgradeYahooCustom.testUpgradeYahooCustom

[GitBox]

sijie opened a new pull request #1772: [tests] Fix integration test 
TestCompatUpgradeYahooCustom.testUpgradeYahooCustom
URL: https://github.com/apache/bookkeeper/pull/1772
 
 
   Descriptions of the changes in this PR:
   
   *Motivation*
   
   apache/bookkeeper#1749 fixes bash script issue but it introduced `set -e` 
which
   will fail `bin/bookkeeper` if grepping configuration file failed.
   
   *Changes*
   
   `set -e` is useful to fail fast if anything fails in the bash script. This 
change
   is to mask exit code of `grep` operation, since the bash script already 
handle
   the grep result.
   
   *Tests*
   
   Manually verified the change works. And the existing 
TestCompatUpgradeYahooCustom.testUpgradeYahooCustom
   integration tests would also verify if the change fixes the problem or not.
   
   > ---
   > In order to uphold a high standard for quality for code contributions, 
Apache BookKeeper runs various precommit
   > checks for pull requests. A pull request can only be merged when it passes 
precommit checks. However running all
   > the precommit checks can take a long time, some trivial changes don't need 
to run all the precommit checks. You
   > can check following list to skip the tests that don't need to run for your 
pull request. Leave them unchecked if
   > you are not sure, committers will help you:
   >
   > - [ ] [skip bookkeeper-server bookie tests]: skip testing 
`org.apache.bookkeeper.bookie` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server client tests]: skip testing 
`org.apache.bookkeeper.client` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server replication tests]: skip testing 
`org.apache.bookkeeper.replication` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server tls tests]: skip testing 
`org.apache.bookkeeper.tls` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server remaining tests]: skip testing all other 
tests in bookkeeper-server module.
   > - [ ] [skip integration tests]: skip docker based integration tests. if 
you make java code changes, you shouldn't skip integration tests.
   > - [ ] [skip build java8]: skip build on java8. *ONLY* skip this when 
*ONLY* changing files under documentation under `site`.
   > - [ ] [skip build java9]: skip build on java9. *ONLY* skip this when 
*ONLY* changing files under documentation under `site`.
   > ---
   
   > ---
   > Be sure to do all of the following to help us incorporate your contribution
   > quickly and easily:
   >
   > If this PR is a BookKeeper Proposal (BP):
   >
   > - [ ] Make sure the PR title is formatted like:
   > `: Description of bookkeeper proposal`
   > `e.g. BP-1: 64 bits ledger is support`
   > - [ ] Attach the master issue link in the description of this PR.
   > - [ ] Attach the google doc link if the BP is written in Google Doc.
   >
   > Otherwise:
   > 
   > - [ ] Make sure the PR title is formatted like:
   > `: Description of pull request`
   > `e.g. Issue 123: Description ...`
   > - [ ] Make sure tests pass via `mvn clean apache-rat:check install 
spotbugs:check`.
   > - [ ] Replace `` in the title with the actual Issue number.
   > 
   > ---
   


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 #1772: [tests] Fix integration test TestCompatUpgradeYahooCustom.testUpgradeYahooCustom

[GitBox]

sijie commented on issue #1772: [tests] Fix integration test 
TestCompatUpgradeYahooCustom.testUpgradeYahooCustom
URL: https://github.com/apache/bookkeeper/pull/1772#issuecomment-434145378
 
 
   @ivankelly can you review this?


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 #1769: ISSUE-1757: prevent race between flush and delete from recreating index

[GitBox]

sijie commented on issue #1769: ISSUE-1757: prevent race between flush and 
delete from recreating index
URL: https://github.com/apache/bookkeeper/pull/1769#issuecomment-434143671
 
 
   rebuild java9


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 #1769: ISSUE-1757: prevent race between flush and delete from recreating index

[GitBox]

sijie commented on a change in pull request #1769: ISSUE-1757: prevent race 
between flush and delete from recreating index
URL: https://github.com/apache/bookkeeper/pull/1769#discussion_r229153973
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
 ##
 @@ -257,13 +260,26 @@ public synchronized void readHeader() throws IOException 
{
 }
 }
 
+public synchronized boolean isDeleted() {
+return deleted;
+}
+
+public static class FileInfoDeleted extends IOException {
 
 Review comment:
   nit: FileInfoDeletedException


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 closed issue #1752: Etcd docker based tests are running on wrong jobs on CI

[GitBox]

sijie closed issue #1752: Etcd docker based tests are running on wrong jobs on 
CI
URL: https://github.com/apache/bookkeeper/issues/1752
 
 
   


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 closed pull request #1766: Issue #1752: Etcd docker based tests are running on wrong jobs on CI

[GitBox]

sijie closed pull request #1766: Issue #1752: Etcd docker based tests are 
running on wrong jobs on CI
URL: https://github.com/apache/bookkeeper/pull/1766
 
 
   

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/.test-infra/jenkins/job_bookkeeper_precommit_integrationtests.groovy 
b/.test-infra/jenkins/job_bookkeeper_precommit_integrationtests.groovy
index 10958ee308..7a169cb9da 100644
--- a/.test-infra/jenkins/job_bookkeeper_precommit_integrationtests.groovy
+++ b/.test-infra/jenkins/job_bookkeeper_precommit_integrationtests.groovy
@@ -60,6 +60,15 @@ freeStyleJob('bookkeeper_precommit_integrationtests') {
 properties(skipTests: true, interactiveMode: false)
 }
 
+// Run metadata driver tests
+maven {
+// Set Maven parameters.
+common_job_properties.setMavenConfig(delegate)
+rootPOM('metadata-drivers/pom.xml')
+goals('-B test -DintegrationTests')
+}
+
+// Run all integration tests
 maven {
 // Set Maven parameters.
 common_job_properties.setMavenConfig(delegate)
diff --git a/.test-infra/scripts/pre-docker-tests.sh 
b/.test-infra/scripts/pre-docker-tests.sh
index 3e92513e1f..1edb48c0d7 100755
--- a/.test-infra/scripts/pre-docker-tests.sh
+++ b/.test-infra/scripts/pre-docker-tests.sh
@@ -29,3 +29,4 @@ docker system prune -f
 # clean up any dangling networks from previous runs
 docker network prune -f --filter name=testnetwork_*
 docker system events > docker.debug-info & echo $! > docker-log.pid
+docker pull quay.io/coreos/etcd:v3.3
diff --git a/metadata-drivers/etcd/pom.xml b/metadata-drivers/etcd/pom.xml
index 0d9e0dd164..8e3b339054 100644
--- a/metadata-drivers/etcd/pom.xml
+++ b/metadata-drivers/etcd/pom.xml
@@ -18,55 +18,76 @@
 http://maven.apache.org/POM/4.0.0;
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
-
-org.apache.bookkeeper.metadata.drivers
-metadata-drivers-parent
-4.9.0-SNAPSHOT
-..
-
-4.0.0
+  
 org.apache.bookkeeper.metadata.drivers
-metadata-stores-etcd
-Apache BookKeeper :: Metadata Drivers:: Etcd
-
-
-org.apache.bookkeeper
-bookkeeper-server
-${project.version}
-
-
-com.coreos
-jetcd-core
-${etcd.version}
-
-
-io.grpc
-*
-
-
-
-
-io.grpc
-grpc-all
-
-
-org.testcontainers
-testcontainers
-test
-
-
-org.apache.bookkeeper
-bookkeeper-common
-${project.version}
-test-jar
-test
-
-
-org.apache.bookkeeper
-bookkeeper-server
-${project.version}
-test-jar
-test
-
-
+metadata-drivers-parent
+4.9.0-SNAPSHOT
+..
+  
+  4.0.0
+  org.apache.bookkeeper.metadata.drivers
+  metadata-stores-etcd
+  Apache BookKeeper :: Metadata Drivers:: Etcd
+  
+
+  org.apache.bookkeeper
+  bookkeeper-server
+  ${project.version}
+
+
+  com.coreos
+  jetcd-core
+  ${etcd.version}
+  
+
+  io.grpc
+  *
+
+  
+
+
+  io.grpc
+  grpc-all
+
+
+  org.testcontainers
+  testcontainers
+  test
+
+
+  org.apache.bookkeeper
+  bookkeeper-common
+  ${project.version}
+  test-jar
+  test
+
+
+  org.apache.bookkeeper
+  bookkeeper-server
+  ${project.version}
+  test-jar
+  test
+
+  
+  
+
+  integrationTests
+  
+
+  integrationTests
+
+  
+  
+
+  
+org.apache.maven.plugins
+maven-surefire-plugin
+
+  false
+
+  
+
+  
+
+  
 


 


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 opened a new pull request #1771: [table service] Fix python client can't read keys written by java clients

[GitBox]

sijie opened a new pull request #1771:  [table service] Fix python client can't 
read keys written by java clients
URL: https://github.com/apache/bookkeeper/pull/1771
 
 
   Descriptions of the changes in this PR:
   
   *Motivation*
   
   Python client is a thin table service client. So it relies on storage 
container
   intercepting the requests to attach routing header. It attaches the routing 
header
   if the requests are needed to route to remote containers; however for the 
local containers
   it doesn't attach the right routing header.
   
   *Changes*
   
   Fix `StorageContainerImpl` to use proxy routing interceptor to intercept 
requests to attach routing header.
   
   *Tests*
   
   Add integration tests in python client & add test for python clients reading 
key/value pairs written by java clients.
   
   > ---
   > In order to uphold a high standard for quality for code contributions, 
Apache BookKeeper runs various precommit
   > checks for pull requests. A pull request can only be merged when it passes 
precommit checks. However running all
   > the precommit checks can take a long time, some trivial changes don't need 
to run all the precommit checks. You
   > can check following list to skip the tests that don't need to run for your 
pull request. Leave them unchecked if
   > you are not sure, committers will help you:
   >
   > - [ ] [skip bookkeeper-server bookie tests]: skip testing 
`org.apache.bookkeeper.bookie` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server client tests]: skip testing 
`org.apache.bookkeeper.client` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server replication tests]: skip testing 
`org.apache.bookkeeper.replication` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server tls tests]: skip testing 
`org.apache.bookkeeper.tls` in bookkeeper-server module.
   > - [ ] [skip bookkeeper-server remaining tests]: skip testing all other 
tests in bookkeeper-server module.
   > - [ ] [skip integration tests]: skip docker based integration tests. if 
you make java code changes, you shouldn't skip integration tests.
   > - [ ] [skip build java8]: skip build on java8. *ONLY* skip this when 
*ONLY* changing files under documentation under `site`.
   > - [ ] [skip build java9]: skip build on java9. *ONLY* skip this when 
*ONLY* changing files under documentation under `site`.
   > ---
   
   > ---
   > Be sure to do all of the following to help us incorporate your contribution
   > quickly and easily:
   >
   > If this PR is a BookKeeper Proposal (BP):
   >
   > - [ ] Make sure the PR title is formatted like:
   > `: Description of bookkeeper proposal`
   > `e.g. BP-1: 64 bits ledger is support`
   > - [ ] Attach the master issue link in the description of this PR.
   > - [ ] Attach the google doc link if the BP is written in Google Doc.
   >
   > Otherwise:
   > 
   > - [ ] Make sure the PR title is formatted like:
   > `: Description of pull request`
   > `e.g. Issue 123: Description ...`
   > - [ ] Make sure tests pass via `mvn clean apache-rat:check install 
spotbugs:check`.
   > - [ ] Replace `` in the title with the actual Issue number.
   > 
   > ---
   


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 #1766: Issue #1752: Etcd docker based tests are running on wrong jobs on CI

[GitBox]

sijie commented on issue #1766: Issue #1752: Etcd docker based tests are 
running on wrong jobs on CI
URL: https://github.com/apache/bookkeeper/pull/1766#issuecomment-434142930
 
 
   IGNORE IT CI


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] athanatos commented on issue #1770: bookie local consistency checker

[GitBox]

athanatos commented on issue #1770: bookie local consistency checker
URL: https://github.com/apache/bookkeeper/issues/1770#issuecomment-434130126
 
 
   #1757 and #1737 were turned up by the protoype so far.
   The underlying index scanning machinery will be used in #1602.


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] athanatos opened a new issue #1770: bookie local consistency checker

[GitBox]

athanatos opened a new issue #1770: bookie local consistency checker
URL: https://github.com/apache/bookkeeper/issues/1770
 
 
   The bookie needs a way to perform online consistency checks within ledger 
storage.  For Sorted/InterleavedLedgerStorage, the prototype has already turned 
up a journal bug and several race conditions.  PR forthcoming.


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] athanatos commented on issue #1769: ISSUE-1757: prevent race between flush and delete from recreating index

[GitBox]

athanatos commented on issue #1769: ISSUE-1757: prevent race between flush and 
delete from recreating index
URL: https://github.com/apache/bookkeeper/pull/1769#issuecomment-434129173
 
 
   retest


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] athanatos opened a new pull request #1769: ISSUE-1757: prevent race between flush and delete from recreating index

[GitBox]

athanatos opened a new pull request #1769: ISSUE-1757: prevent race between 
flush and delete from recreating index
URL: https://github.com/apache/bookkeeper/pull/1769
 
 
   IndexPersistencManager.flushLedgerHandle can race with delete by
   obtaining a FileInfo just prior to delete and then proceeding to rewrite
   the file info resurrecting it.  FileInfo provides a convenient point of
   synchronization for avoinding this race.  FileInfo.moveLedgerIndexFile,
   FileInfo.flushHeader, and FileInfo.delete() are synchronized already, so
   this patch simply adds a deleted flag to the FileInfo object to indicate
   that the FileInfo has become invalid.  checkOpen is called in every
   method and will now throw FileInfoDeleted if delete has been called.
   IndexPersistenceManager can catch it and deal with it appropriately in
   flush (which generally means moving onto the next ledger).
   
   This patch also eliminates ledgersToFlush and ledgersFlushing.  Their
   purpose appears to be to allow delete to avoid flushing a ledger which
   has been selected for flushing but not flushed yet avoiding the above
   race.  It's not sufficient, however, because IndexInMemPageMgr calls
   IndexPersistenceManager.flushLedgerHeader, which can obtain a FileInfo
   for the ledger prior to the deletion and then call
   relocateIndexFileAndFlushHeader afterwards.
   
   Also, if the purpose was to avoid concurrent calls into
   flushSpecificLedger on the same ledger, it's wrong because of the
   following sequence:
   
   t0: thread 0 calls flushOneOrMoreLedgers
   t1: thread 0 place ledger 10 into ledgersFlushing and completes
   flushSpecificLedger
   t2: thread 2 performs a write to ledger 10
   t3: thread 1 calls flushOneOrMoreLedgers skipping ledger 10
   t4: thread 0 releases ledger 10 from ledgersFlushing
   t5: thread 1 completes flushOneOrMoreLedgers
   
   Although thread 1 begins to flush after the write to ledger 10, it won't
   capture the write rendering the flush incorrect.  I don't think it's
   actually worth avoiding overlapping flushes here because both FileInfo
   and LedgerEntryPage track dirty state.  As such, it seems simpler to
   just get rid of them.
   
   This patch also Adds a more agressive version of testFlushDeleteRace to
   test the new behavior.  Testing with multiple flushers turned up a bug
   with LedgerEntryPage.getPageToWrite where didn't return a buffer with
   independent read pointers, so this patch addresses that as well.
   
   (@bug W-5549455@)
   (@rev cguttapalem@)
   Signed-off-by: Samuel Just 
   (cherry picked from commit 7b5ac3d5e76ac4df618764cafe80aef2994703ec)
   


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] athanatos closed pull request #1768: ISSUE-1757: prevent race between flush and delete from recreating index

[GitBox]

athanatos closed pull request #1768: ISSUE-1757: prevent race between flush and 
delete from recreating index
URL: https://github.com/apache/bookkeeper/pull/1768
 
 
   

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/bookie/FileInfo.java 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
index a5ddacf0ff..c38bb3795d 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
@@ -101,6 +101,8 @@
 // this FileInfo Header Version
 int headerVersion;
 
+private boolean deleted;
+
 public FileInfo(File lf, byte[] masterKey, int fileInfoVersionToWrite) 
throws IOException {
 super(WATCHER_RECYCLER);
 
@@ -108,6 +110,7 @@ public FileInfo(File lf, byte[] masterKey, int 
fileInfoVersionToWrite) throws IO
 this.masterKey = masterKey;
 mode = "rw";
 this.headerVersion = fileInfoVersionToWrite;
+this.deleted = false;
 }
 
 synchronized Long getLastAddConfirmed() {
@@ -257,6 +260,16 @@ public synchronized void readHeader() throws IOException {
 }
 }
 
+public synchronized boolean isDeleted() {
+return deleted;
+}
+
+public class FileInfoDeleted extends IOException {
+FileInfoDeleted() {
+super("FileInfo already deleted");
+}
+}
+
 @VisibleForTesting
 void checkOpen(boolean create) throws IOException {
 checkOpen(create, false);
@@ -264,6 +277,9 @@ void checkOpen(boolean create) throws IOException {
 
 private synchronized void checkOpen(boolean create, boolean 
openBeforeClose)
 throws IOException {
+if (deleted) {
+throw new FileInfoDeleted();
+}
 if (fc != null) {
 return;
 }
@@ -540,6 +556,7 @@ public synchronized void moveToNewLocation(File newFile, 
long size) throws IOExc
 }
 
 public synchronized boolean delete() {
+deleted = true;
 return lf.delete();
 }
 
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfoBackingCache.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfoBackingCache.java
index 6beba6a744..078292fb81 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfoBackingCache.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfoBackingCache.java
@@ -49,6 +49,8 @@ private static CachedFileInfo 
tryRetainFileInfo(CachedFileInfo fi) throws IOExce
 boolean retained = fi.tryRetain();
 if (!retained) {
 throw new IOException("FileInfo " + fi + " is already marked 
dead");
+} else if (fi.isDeleted()) {
+throw new Bookie.NoLedgerException(fi.ledgerId);
 }
 return fi;
 }
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexInMemPageMgr.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexInMemPageMgr.java
index 66e97f7942..0cf5cc93f8 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexInMemPageMgr.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexInMemPageMgr.java
@@ -40,7 +40,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.bookkeeper.conf.ServerConfiguration;
@@ -345,12 +344,6 @@ public void onSetDirty(LedgerEntryPage lep) {
 // flush and read pages
 private final IndexPersistenceMgr indexPersistenceManager;
 
-/**
- * the list of potentially dirty ledgers.
- */
-private final ConcurrentLinkedQueue ledgersToFlush = new 
ConcurrentLinkedQueue();
-private final ConcurrentSkipListSet ledgersFlushing = new 
ConcurrentSkipListSet();
-
 // Stats
 private final Counter ledgerCacheHitCounter;
 private final Counter ledgerCacheMissCounter;
@@ -504,7 +497,6 @@ private LedgerEntryPage grabLedgerEntryPage(long ledger, 
long pageEntry) throws
 
 void removePagesForLedger(long ledgerId) {
 pageMapAndList.removeEntriesForALedger(ledgerId);
-ledgersToFlush.remove(ledgerId);
 }
 
 long getLastEntryInMem(long ledgerId) {
@@ -542,18 +534,12 @@ private LedgerEntryPage grabCleanPage(long ledger, long 
entry) throws IOExceptio
 }
 
 void flushOneOrMoreLedgers(boolean doAll) throws IOException {
-if (ledgersToFlush.isEmpty()) {
-

[GitHub] athanatos opened a new pull request #1768: ISSUE-1757: prevent race between flush and delete from recreating index

[GitBox]

athanatos opened a new pull request #1768: ISSUE-1757: prevent race between 
flush and delete from recreating index
URL: https://github.com/apache/bookkeeper/pull/1768
 
 
   IndexPersistencManager.flushLedgerHandle can race with delete by
   obtaining a FileInfo just prior to delete and then proceeding to rewrite
   the file info resurrecting it.  FileInfo provides a convenient point of
   synchronization for avoiding this race.  FileInfo.moveLedgerIndexFile,
   FileInfo.flushHeader, and FileInfo.delete() are synchronized already, so
   this patch simply adds a deleted flag to the FileInfo object to indicate
   that the FileInfo has become invalid.  checkOpen is called in every
   method and will now throw FileInfoDeleted if delete has been called.
   IndexPersistenceManager can catch it and deal with it appropriately in
   flush (which generally means moving onto the next ledger).
   
   This patch also eliminates ledgersToFlush and ledgersFlushing.  Their
   purpose appears to be to allow delete to avoid flushing a ledger which
   has been selected for flushing but not flushed yet avoiding the above
   race.  It's not sufficient, however, because IndexInMemPageMgr calls
   IndexPersistenceManager.flushLedgerHeader, which can obtain a FileInfo
   for the ledger prior to the deletion and then call
   relocateIndexFileAndFlushHeader afterwards.
   
   Also, if the purpose was to avoid concurrent calls into
   flushSpecificLedger on the same ledger, it's wrong because of the
   following sequence:
   
   t0: thread 0 calls flushOneOrMoreLedgers
   t1: thread 0 place ledger 10 into ledgersFlushing and completes
   flushSpecificLedger
   t2: thread 2 performs a write to ledger 10
   t3: thread 1 calls flushOneOrMoreLedgers skipping ledger 10
   t4: thread 0 releases ledger 10 from ledgersFlushing
   t5: thread 1 completes flushOneOrMoreLedgers
   
   Although thread 1 begins to flush after the write to ledger 10, it won't
   capture the write rendering the flush incorrect.  I don't think it's
   actually worth avoiding overlapping flushes here because both FileInfo
   and LedgerEntryPage track dirty state.  As such, it seems simpler to
   just get rid of them.
   
   This patch also Adds a more aggressive version of testFlushDeleteRace to
   test the new behavior.  Testing with multiple flushers turned up a bug
   with LedgerEntryPage.getPageToWrite where didn't return a buffer with
   independent read pointers, so this patch addresses that as well.
   
   (@bug W-5549455@)
   (@rev cguttapalem@)
   Signed-off-by: Samuel Just 
   (cherry picked from commit 7b5ac3d5e76ac4df618764cafe80aef2994703ec)


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 opened a new issue #1767: security vulnerabilities in 3rd party dependencies

[GitBox]

sijie opened a new issue #1767: security vulnerabilities in 3rd party 
dependencies
URL: https://github.com/apache/bookkeeper/issues/1767
 
 
   Similar as apache/pulsar#2882, bookkeeper has 3rd party dependencies that 
are exposed to security vulnerabilities
   
   ```
   mvn com.redhat.victims.maven:security-versions:check
   ```
   
   Results:
   
   ```
   [ERROR] jline:jline is vulnerable to CVE-2013-2035
   [ERROR] com.fasterxml.jackson.core:jackson-databind is vulnerable to 
CVE-2017-17485
   [ERROR] com.fasterxml.jackson.core:jackson-databind is vulnerable to 
CVE-2017-7525
   [ERROR] com.fasterxml.jackson.core:jackson-databind is vulnerable to 
CVE-2018-5968
   ```


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 #1766: Issue #1752: Etcd docker based tests are running on wrong jobs on CI

[GitBox]

sijie commented on issue #1766: Issue #1752: Etcd docker based tests are 
running on wrong jobs on CI
URL: https://github.com/apache/bookkeeper/pull/1766#issuecomment-434056064
 
 
   @eolivelli integration tests was broken at handling yahoo custom image. Ivan 
and me are looking into it. so you can ignore it ci to merge the PR


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 #1766: Issue #1752: Etcd docker based tests are running on wrong jobs on CI

[GitBox]

eolivelli commented on issue #1766: Issue #1752: Etcd docker based tests are 
running on wrong jobs on CI
URL: https://github.com/apache/bookkeeper/pull/1766#issuecomment-434042803
 
 
   rerun integration tests


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 #1756: [Question] metadataServiceUri with multiple zk servers

[GitBox]

sijie commented on issue #1756: [Question] metadataServiceUri with multiple zk 
servers
URL: https://github.com/apache/bookkeeper/issues/1756#issuecomment-434033302
 
 
   @eolivelli I think the question how current code extracts zk servers from 
metadata service uri. I know dlog uri handles ';' and converts it to ','. but I 
am not very sure about the new metadata service uri, have to look more into 
this.


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 #1671: remove misleading comment

[GitBox]

sijie commented on issue #1671: remove misleading comment
URL: https://github.com/apache/bookkeeper/pull/1671#issuecomment-434032480
 
 
   ping @ivankelly 


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 #1766: Issue #1752: Etcd docker based tests are running on wrong jobs on CI

[GitBox]

sijie commented on issue #1766: Issue #1752: Etcd docker based tests are 
running on wrong jobs on CI
URL: https://github.com/apache/bookkeeper/pull/1766#issuecomment-434032265
 
 
   @eolivelli @codingwangqi yes the jenkins changes are only applied when this 
PR is merged.


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 back to stable : bookkeeper_postcommit_master_java8 #302

[Apache Jenkins Server]

See 

Jenkins build became unstable: bookkeeper_release_branch_47_java9 #202

[Apache Jenkins Server]

See 

Jenkins build became unstable: bookkeeper_release_branch_46 #200

[Apache Jenkins Server]

See 

Build failed in Jenkins: bookkeeper_release_branch_48_integrationtests #75

[Apache Jenkins Server]

See 


Changes:

[sijie] Don't cache Bookie hostname DNS resolution forever

--
[...truncated 1.31 MB...]
---
 T E S T S
---
Running org.apache.bookkeeper.tests.backwardcompat.TestCompatRecoveryNoPassword
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 25.078 sec

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

2018-10-29\T\13:06:02.416 [INFO] 
2018-10-29\T\13:06:02.416 [INFO] 

2018-10-29\T\13:06:02.416 [INFO] Building Apache BookKeeper :: Tests :: 
Backward Compatibility :: Test upgrade 4.1.0 to current in cluster with cookies 
4.8.0-SNAPSHOT
2018-10-29\T\13:06:02.416 [INFO] 

2018-10-29\T\13:06:02.461 [INFO] 
2018-10-29\T\13:06:02.461 [INFO] --- 
groovy-eclipse-compiler:2.9.2-04:add-groovy-build-paths 
(default-add-groovy-build-paths) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:02.461 [INFO] Adding /src/main/groovy to the list of source 
folders
2018-10-29\T\13:06:02.461 [INFO] Adding /src/test/groovy to the list of test 
source folders
2018-10-29\T\13:06:02.462 [INFO] 
2018-10-29\T\13:06:02.462 [INFO] --- maven-remote-resources-plugin:1.5:process 
(process-resource-bundles) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:04.210 [INFO] 
2018-10-29\T\13:06:04.210 [INFO] --- maven-resources-plugin:2.7:resources 
(default-resources) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:04.212 [INFO] Using 'UTF-8' encoding to copy filtered 
resources.
2018-10-29\T\13:06:04.212 [INFO] skip non existing resourceDirectory 

2018-10-29\T\13:06:04.212 [INFO] Copying 3 resources
2018-10-29\T\13:06:04.213 [INFO] 
2018-10-29\T\13:06:04.213 [INFO] --- maven-compiler-plugin:3.7.0:compile 
(default-compile) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:04.214 [INFO] No sources to compile
2018-10-29\T\13:06:04.215 [INFO] 
2018-10-29\T\13:06:04.215 [INFO] --- maven-resources-plugin:2.7:testResources 
(default-testResources) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:04.215 [INFO] Using 'UTF-8' encoding to copy filtered 
resources.
2018-10-29\T\13:06:04.215 [INFO] Copying 1 resource
2018-10-29\T\13:06:04.216 [INFO] Copying 3 resources
2018-10-29\T\13:06:04.217 [INFO] 
2018-10-29\T\13:06:04.217 [INFO] --- maven-compiler-plugin:3.7.0:testCompile 
(default-testCompile) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:04.221 [INFO] Changes detected - recompiling the module!
2018-10-29\T\13:06:04.223 [INFO] Using Groovy-Eclipse compiler to compile both 
Java and Groovy files
2018-10-29\T\13:06:04.467 [INFO] 
2018-10-29\T\13:06:04.467 [INFO] --- maven-surefire-plugin:2.8.1:test 
(default-test) @ old-cookie-new-cluster ---
2018-10-29\T\13:06:04.470 [INFO] Surefire report directory: 


---
 T E S T S
---
Running 
org.apache.bookkeeper.tests.backwardcompat.TestCompatUpgradeOldServerInClusterWithCookies
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 66.561 sec

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

2018-10-29\T\13:07:45.495 [INFO] 
2018-10-29\T\13:07:45.496 [INFO] 

2018-10-29\T\13:07:45.496 [INFO] Building Apache BookKeeper :: Tests :: 
Backward Compatibility :: Test old clients working on current server 
4.8.0-SNAPSHOT
2018-10-29\T\13:07:45.496 [INFO] 

2018-10-29\T\13:07:45.540 [INFO] 
2018-10-29\T\13:07:45.540 [INFO] --- 
groovy-eclipse-compiler:2.9.2-04:add-groovy-build-paths 
(default-add-groovy-build-paths) @ backward-compat-current-server-old-clients 
---
2018-10-29\T\13:07:45.540 [INFO] Adding /src/main/groovy to the list of source 
folders
2018-10-29\T\13:07:45.540 [INFO] Adding /src/test/groovy to the list of test 
source folders
2018-10-29\T\13:07:45.541 [INFO] 
2018-10-29\T\13:07:45.541 [INFO] --- maven-remote-resources-plugin:1.5:process 
(process-resource-bundles) @ backward-compat-current-server-old-clients ---
2018-10-29\T\13:07:47.184 [INFO] 
2018-10-29\T\13:07:47.184 [INFO] --- maven-resources-plugin:2.7:resources 
(default-resources) @ backward-compat-current-server-old-clients ---
2018-10-29\T\13:07:47.185 [INFO] Using 'UTF-8' encoding to copy filtered 
resources.
2018-10-29\T\13:07:47.185 [INFO] skip non existing 

Jenkins build is back to normal : bookkeeper_release_branch_47_integrationtests #190

[Apache Jenkins Server]

See 

[GitHub] eolivelli commented on issue #1756: [Question] metadataServiceUri with multiple zk servers

[GitBox]

eolivelli commented on issue #1756: [Question] metadataServiceUri with multiple 
zk servers
URL: https://github.com/apache/bookkeeper/issues/1756#issuecomment-433811177
 
 
   In fact the code which translates from the legacy zkServer to the new model 
is
   
https://github.com/apache/bookkeeper/blob/d9444ba143c9cdba4af427afdd9f0b696c1d7683/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java#L238
   
   and it simply concatenates:
   - metadata protocol
   - list of zk servers
   - ledgers path
   
   


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 #1756: [Question] metadataServiceUri with multiple zk servers

[GitBox]

sijie commented on issue #1756: [Question] metadataServiceUri with multiple zk 
servers
URL: https://github.com/apache/bookkeeper/issues/1756#issuecomment-433810819
 
 
   @hiscal2015 hmm, I think ',' can not be used in a java URI. URI will take 
characters before ','. That might example why you see 
`zk://server1:port,server2:port,server3:port/ledgers` doesn't work and 
`zk+hierarchical://server1:2181/ledgers,zk+hierarchical://server2:2181/ledgers,zk+hierarchical://server3:2181/ledgers`
 works.
   
   Can you try `zk://server1:port;server2:port;server3:port/ledgers`? 
   
   > Looks like the setting is only available on bookie, client only needs 
webservice and broker url.
   
   from pulsar's perspective, it is a setting for broker and bookies.


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 #1756: [Question] metadataServiceUri with multiple zk servers

[GitBox]

eolivelli commented on issue #1756: [Question] metadataServiceUri with multiple 
zk servers
URL: https://github.com/apache/bookkeeper/issues/1756#issuecomment-433810757
 
 
   @hiscal2015 metadataServiceUri is available on client as well.
   Are you using directly a client or another application ?


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] hiscal2015 commented on issue #1756: [Question] metadataServiceUri with multiple zk servers

[GitBox]

hiscal2015 commented on issue #1756: [Question] metadataServiceUri with 
multiple zk servers
URL: https://github.com/apache/bookkeeper/issues/1756#issuecomment-433807510
 
 
   Looks like the setting is only available on bookie, client only needs 
webservice and broker url.


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 #1756: [Question] metadataServiceUri with multiple zk servers

[GitBox]

eolivelli commented on issue #1756: [Question] metadataServiceUri with multiple 
zk servers
URL: https://github.com/apache/bookkeeper/issues/1756#issuecomment-433806689
 
 
   This sounds to me as a big problem.
   I have to performs trials and reproduce your case.
   Are you using that config both on bookie and client?


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