[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387640856
 
 
   @jvrao I have updated the patch with the assertions, let's wait for Sijie's 
(and eventually Ivan) final comments before 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] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387687819
 
 
   retest this please


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

Build failed in Jenkins: bookkeeper_postcommit_master_java8 #127

2018-05-09 Thread Apache Jenkins Server 
See 


--
[...truncated 351.95 KB...]
2018-05-09T12:14:37.137 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/antlr/antlr/2.7.7/antlr-2.7.7.jar
2018-05-09T12:14:37.137 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/antlr/antlr4-runtime/4.5.3/antlr4-runtime-4.5.3.jar
2018-05-09T12:14:37.138 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar
2018-05-09T12:14:37.138 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-cli/commons-cli/1.3.1/commons-cli-1.3.1.jar
2018-05-09T12:14:37.157 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-cli/commons-cli/1.3.1/commons-cli-1.3.1.jar
 (53 kB at 2.5 MB/s)
2018-05-09T12:14:37.157 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/com/google/guava/guava/19.0/guava-19.0.jar
2018-05-09T12:14:37.175 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar
 (234 kB at 5.8 MB/s)
2018-05-09T12:14:37.175 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/reporting/maven-reporting-impl/2.3/maven-reporting-impl-2.3.jar
2018-05-09T12:14:37.188 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/antlr/antlr4-runtime/4.5.3/antlr4-runtime-4.5.3.jar
 (302 kB at 5.7 MB/s)
2018-05-09T12:14:37.188 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-utils/0.6/maven-shared-utils-0.6.jar
2018-05-09T12:14:37.192 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/reporting/maven-reporting-impl/2.3/maven-reporting-impl-2.3.jar
 (18 kB at 327 kB/s)
2018-05-09T12:14:37.192 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-digester/commons-digester/1.6/commons-digester-1.6.jar
2018-05-09T12:14:37.203 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/antlr/antlr/2.7.7/antlr-2.7.7.jar (445 kB 
at 6.5 MB/s)
2018-05-09T12:14:37.203 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
2018-05-09T12:14:37.216 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
 (11 kB at 142 kB/s)
2018-05-09T12:14:37.216 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
2018-05-09T12:14:37.229 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
 (11 kB at 123 kB/s)
2018-05-09T12:14:37.229 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
2018-05-09T12:14:37.229 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-utils/0.6/maven-shared-utils-0.6.jar
 (165 kB at 1.8 MB/s)
2018-05-09T12:14:37.229 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
2018-05-09T12:14:37.236 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-digester/commons-digester/1.6/commons-digester-1.6.jar
 (168 kB at 1.7 MB/s)
2018-05-09T12:14:37.236 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
2018-05-09T12:14:37.244 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
 (53 kB at 496 kB/s)
2018-05-09T12:14:37.244 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
2018-05-09T12:14:37.245 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
 (61 kB at 567 kB/s)
2018-05-09T12:14:37.245 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-integration-tools/1.6/doxia-integration-tools-1.6.jar
2018-05-09T12:14:37.252 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
 (15 kB at 134 kB/s)
2018-05-09T12:14:37.253 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/codehaus/plexus/plexus-container-default/1.0-alpha-9/plexus-container-default-1.0-alpha-9.jar
2018-05-09T12:14:37.259 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
 (38 kB at 310 kB/s)
2018-05-09T12:14:37.260 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/codehaus/plexus/plexus-utils/3.0.24/plexus-utils-3.0.24.jar
2018-05-09T12:14:37.269 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/m

Build failed in Jenkins: bookkeeper_release_branch_47_java9 #30

2018-05-09 Thread Apache Jenkins Server 
See 


--
[...truncated 352.80 KB...]
2018-05-09T12:35:36.011 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/antlr/antlr4-runtime/4.5.3/antlr4-runtime-4.5.3.jar
2018-05-09T12:35:36.012 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar
2018-05-09T12:35:36.013 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-cli/commons-cli/1.3.1/commons-cli-1.3.1.jar
2018-05-09T12:35:36.033 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-cli/commons-cli/1.3.1/commons-cli-1.3.1.jar
 (53 kB at 2.4 MB/s)
2018-05-09T12:35:36.034 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/com/google/guava/guava/19.0/guava-19.0.jar
2018-05-09T12:35:36.057 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar
 (234 kB at 3.8 MB/s)
2018-05-09T12:35:36.057 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/reporting/maven-reporting-impl/2.3/maven-reporting-impl-2.3.jar
2018-05-09T12:35:36.062 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/antlr/antlr4-runtime/4.5.3/antlr4-runtime-4.5.3.jar
 (302 kB at 4.4 MB/s)
2018-05-09T12:35:36.062 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-utils/0.6/maven-shared-utils-0.6.jar
2018-05-09T12:35:36.070 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/reporting/maven-reporting-impl/2.3/maven-reporting-impl-2.3.jar
 (18 kB at 310 kB/s)
2018-05-09T12:35:36.070 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-digester/commons-digester/1.6/commons-digester-1.6.jar
2018-05-09T12:35:36.077 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/antlr/antlr/2.7.7/antlr-2.7.7.jar (445 kB 
at 5.2 MB/s)
2018-05-09T12:35:36.077 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
2018-05-09T12:35:36.088 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
 (11 kB at 147 kB/s)
2018-05-09T12:35:36.088 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
2018-05-09T12:35:36.096 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-utils/0.6/maven-shared-utils-0.6.jar
 (165 kB at 2.0 MB/s)
2018-05-09T12:35:36.096 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
2018-05-09T12:35:36.100 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
 (11 kB at 129 kB/s)
2018-05-09T12:35:36.100 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
2018-05-09T12:35:36.102 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-digester/commons-digester/1.6/commons-digester-1.6.jar
 (168 kB at 1.9 MB/s)
2018-05-09T12:35:36.103 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
2018-05-09T12:35:36.113 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
 (61 kB at 607 kB/s)
2018-05-09T12:35:36.113 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
2018-05-09T12:35:36.116 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
 (53 kB at 515 kB/s)
2018-05-09T12:35:36.116 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-integration-tools/1.6/doxia-integration-tools-1.6.jar
2018-05-09T12:35:36.124 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
 (15 kB at 139 kB/s)
2018-05-09T12:35:36.124 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/codehaus/plexus/plexus-resources/1.0.1/plexus-resources-1.0.1.jar
2018-05-09T12:35:36.127 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
 (38 kB at 332 kB/s)
2018-05-09T12:35:36.127 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/codehaus/plexus/plexus-interpolation/1.21/plexus-interpolation-1.21.jar
2018-05-09T12:35:36.131 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-integration-tools/1.6/doxia-integration-tools-1.6.jar
 (45 kB at 378 kB/s)
2018-05-09T12:35:36.131 [INFO] Downloadin

Build failed in Jenkins: bookkeeper_postcommit_master_java9 #126

2018-05-09 Thread Apache Jenkins Server 
See 


--
[...truncated 352.40 KB...]
2018-05-09T12:52:51.905 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/antlr/antlr4-runtime/4.5.3/antlr4-runtime-4.5.3.jar
2018-05-09T12:52:51.905 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/antlr/antlr/2.7.7/antlr-2.7.7.jar
2018-05-09T12:52:51.913 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar
2018-05-09T12:52:51.914 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-cli/commons-cli/1.3.1/commons-cli-1.3.1.jar
2018-05-09T12:52:51.935 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-cli/commons-cli/1.3.1/commons-cli-1.3.1.jar
 (53 kB at 1.4 MB/s)
2018-05-09T12:52:51.935 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/com/google/guava/guava/19.0/guava-19.0.jar
2018-05-09T12:52:51.964 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar
 (234 kB at 3.5 MB/s)
2018-05-09T12:52:51.964 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/reporting/maven-reporting-impl/2.3/maven-reporting-impl-2.3.jar
2018-05-09T12:52:51.979 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/reporting/maven-reporting-impl/2.3/maven-reporting-impl-2.3.jar
 (18 kB at 246 kB/s)
2018-05-09T12:52:51.979 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-utils/0.6/maven-shared-utils-0.6.jar
2018-05-09T12:52:51.985 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/antlr/antlr4-runtime/4.5.3/antlr4-runtime-4.5.3.jar
 (302 kB at 3.2 MB/s)
2018-05-09T12:52:51.986 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/commons-digester/commons-digester/1.6/commons-digester-1.6.jar
2018-05-09T12:52:52.007 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/antlr/antlr/2.7.7/antlr-2.7.7.jar (445 kB 
at 3.9 MB/s)
2018-05-09T12:52:52.007 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
2018-05-09T12:52:52.020 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
 (11 kB at 105 kB/s)
2018-05-09T12:52:52.020 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
2018-05-09T12:52:52.023 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-utils/0.6/maven-shared-utils-0.6.jar
 (165 kB at 1.5 MB/s)
2018-05-09T12:52:52.023 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
2018-05-09T12:52:52.026 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/commons-digester/commons-digester/1.6/commons-digester-1.6.jar
 (168 kB at 1.5 MB/s)
2018-05-09T12:52:52.026 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
2018-05-09T12:52:52.032 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
 (11 kB at 95 kB/s)
2018-05-09T12:52:52.032 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
2018-05-09T12:52:52.042 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
 (53 kB at 411 kB/s)
2018-05-09T12:52:52.042 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
2018-05-09T12:52:52.046 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
 (61 kB at 461 kB/s)
2018-05-09T12:52:52.046 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-integration-tools/1.6/doxia-integration-tools-1.6.jar
2018-05-09T12:52:52.050 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
 (15 kB at 112 kB/s)
2018-05-09T12:52:52.050 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/codehaus/plexus/plexus-container-default/1.0-alpha-9/plexus-container-default-1.0-alpha-9.jar
2018-05-09T12:52:52.056 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
 (38 kB at 265 kB/s)
2018-05-09T12:52:52.056 [INFO] Downloading: 
https://repo.maven.apache.org/maven2/org/codehaus/plexus/plexus-utils/3.0.24/plexus-utils-3.0.24.jar
2018-05-09T12:52:52.066 [INFO] Downloaded: 
https://repo.maven.apache.org/maven2/org/apache/ma

Jenkins build is back to normal : bookkeeper_release_branch_47_integrationtests #20

2018-05-09 Thread Apache Jenkins Server 
See

Jenkins build became unstable: bookkeeper_codecoverage #77

2018-05-09 Thread Apache Jenkins Server 
See

Jenkins build became unstable: bookkeeper_release_branch_47_java8 #30

2018-05-09 Thread Apache Jenkins Server 
See

[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387775537
 
 
   retest this please


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 #1371: User isEmpty method instead of size check

2018-05-09 Thread GitBox 
eolivelli commented on issue #1371: User isEmpty method instead of size check
URL: https://github.com/apache/bookkeeper/pull/1371#issuecomment-387779886
 
 
   retest this please


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 issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2

2018-05-09 Thread GitBox 
jvrao commented on issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet 
doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2
URL: https://github.com/apache/bookkeeper/issues/1380#issuecomment-387819632
 
 
   I think @reddycharan 's point is very simple.
   
   1. The logic of checkCovered() is not correct.
   ```
 if (nodesNotCovered >= ackQuorumSize || (nodesOkay == 0 && 
nodesUninitialized > 0)) {
   return false;
  }
   ```
 The negative logic is not correct. We should be checking
   ```
   
   if (nodesOK >= Ack ) {
 return True;
   } else {
return  false;
   }
   ```
   2. But if we look at the usage of this, it is used only in readLac. So as 
far as ReadLac, we are OK as long as we have one valid response from that write 
quorum. If that is agreed upon behavior, do we really need a class and 
checkCovered() response?
   
   @sijie @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] jvrao commented on issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2

2018-05-09 Thread GitBox 
jvrao commented on issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet 
doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2
URL: https://github.com/apache/bookkeeper/issues/1380#issuecomment-387819632
 
 
   I think @reddycharan 's point is very simple.
   
   1. The logic of checkCovered() is not correct.
   ```
 if (nodesNotCovered >= ackQuorumSize || (nodesOkay == 0 && 
nodesUninitialized > 0)) {
   return false;
  }
   ```
 The negative logic is not correct. We should be checking
   ```
   
   if (nodesOK >= AckQ ) {
 return True;
   } else {
return  false;
   }
   ```
   2. Being said that, if we look at the usage of this, it is used only in 
readLac. So as far as ReadLac, we are OK as long as we have one valid response 
from that write quorum. If that is agreed upon behavior, do we really need a 
class and checkCovered() response?
   
   @sijie @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] reddycharan commented on issue #1397: Bookies should be from different racks in a Writequorum.

2018-05-09 Thread GitBox 
reddycharan commented on issue #1397: Bookies should be from different racks in 
a Writequorum.
URL: https://github.com/apache/bookkeeper/pull/1397#issuecomment-387831644
 
 
   retest this please
   


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] reddycharan commented on issue #1391: Issue #570: EntryLogManagerForEntryLogPerLedger implementation

2018-05-09 Thread GitBox 
reddycharan commented on issue #1391: Issue #570: 
EntryLogManagerForEntryLogPerLedger implementation
URL: https://github.com/apache/bookkeeper/pull/1391#issuecomment-387831744
 
 
   retest this please


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] arunlakshman opened a new pull request #1398: ISSUE #1339: Cleanup the directories created by DbLedgerStorageTest

2018-05-09 Thread GitBox 
arunlakshman opened a new pull request #1398: ISSUE #1339: Cleanup the 
directories created by DbLedgerStorageTest
URL: https://github.com/apache/bookkeeper/pull/1398
 
 
   Descriptions of the changes in this PR:
   
 Cleaned up the directories `dir1` and `dir2` left behind by the test 
`DbLedgerStorageTest`
   
   Master Issue: #1339 
   
   > ---
   > 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 #1380: addBookieAndCheckCovered in RRQuorumCoverageSet doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2

2018-05-09 Thread GitBox 
sijie commented on issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet 
doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2
URL: https://github.com/apache/bookkeeper/issues/1380#issuecomment-387961241
 
 
   @jvrao @reddycharan : 
   
   because the way how bookkeeper writes entries and the lac protocol. we don't 
really need "quorum" read for reading any entries. that says the coverage here 
is never designed for a "quorum-read" on entries. so there is no point saying 
"checkCovered() is not correct". 
   
   we only need to quorum coverage check for LAC on determining what is the LAC 
and that is the class designed for. 
   
   > o as far as ReadLac, we are OK as long as we have one valid response from 
that write quorum. If that is agreed upon behavior, do we really need a class 
and checkCovered() response?
   
   if the goal is just to read any valid lac, we don't need quorum check; if 
the goal is to read max valid lac, the quorum check is needed (however I should 
also say it is not that strongly required). if you look into the API, read any 
valid lac is tryReadLastAddConfirmed, while read max valid lac is 
readLastAddConfirmed.
   
   Hope this make the question clear.


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 #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387961684
 
 
   retest this please


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 #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387961769
 
 
   I kicked tests again, the error seemed not related to the patch ("no enough 
native threads"...)


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 #1398: ISSUE #1339: Cleanup the directories created by DbLedgerStorageTest

2018-05-09 Thread GitBox 
sijie commented on a change in pull request #1398: ISSUE #1339: Cleanup the 
directories created by DbLedgerStorageTest
URL: https://github.com/apache/bookkeeper/pull/1398#discussion_r187247855
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java
 ##
 @@ -241,16 +242,20 @@ public void testBookieCompaction() throws Exception {
 @Test
 public void doubleDirectory() throws Exception {
 int gcWaitTime = 1000;
+File firstDir = new File("dir1");
 
 Review comment:
   nit: a better approach would be create the subdir under the tmpDir 
initialized in this test suite.
   
   ```
   File firstDir = new File(tmpDir, "dir1");
   File secondDir = new File(tmpDir, "dir2");
   ```


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 #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387962407
 
 
   @sijie  @jvrao since these "names" (forceLedger vs sync) are only on bare 
code, not on public client API and not on wire protocol, I would like to merge 
this patch now so that bookie side changes are totally in
   this way I can move forwards with steps on client: we cannot merge stuff on 
client if server side stuff is not complete
   
   I can send a follow up change to rename variables and methods if we feel 
strong about changing *force* to *sync*.
   
   Do this sound reasonable to you ?


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 #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
sijie commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387964478
 
 
   @jvrao I think we are using "force" to follow the convention in file channel 
java api - 
https://docs.oracle.com/javase/7/docs/api/java/nio/channels/FileChannel.html#force(boolean)
   
   @eolivelli I understand you want to move this forward. but we need to wait 
until other people's replies. If there are not replies at a certain time, feel 
free to move forward. 


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 #1393: BP-14 forceLedger wire protocol server side implementation

2018-05-09 Thread GitBox 
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side 
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387964825
 
 
   @sijie I will stay tuned.
   
   I apologize to be annoying on this topic.


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 #1397: Bookies should be from different racks in a Writequorum.

2018-05-09 Thread GitBox 
sijie commented on a change in pull request #1397: Bookies should be from 
different racks in a Writequorum.
URL: https://github.com/apache/bookkeeper/pull/1397#discussion_r187251645
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java
 ##
 @@ -542,9 +544,58 @@ public void 
handleBookiesThatJoined(Set joinedBookies) {
 curRack = localNode.getNetworkLocation();
 }
 } else {
-curRack = "~" + prevNode.getNetworkLocation();
+StringBuilder sb = new StringBuilder();
+sb.append("~");
+
+if (writeQuorumSize > 1) {
+/*
+ * Ideally RackAwareEnsemblePlacementPolicy should try
+ * to select bookies from different racks for a write
+ * quorum. So in a WriteQuorum, bookies should be from
+ * WriteQuorum number of racks. So we would add racks 
of
+ * (WriteQuorumSize-1) neighbours (both sides) to the
+ * exclusion list (~curRack).
+ */
+for (int j = 1; j < writeQuorumSize; j++) {
 
 Review comment:
   can we have a separate setting like `minNumRacksPerWriteQuorum` and by 
default set it to 2? so that it doesn't change the default behavior here.
   
   because not everyone has enough rack diversity. especially when running 
bookkeeper in cloud, we only can use `availability zone` as `rack`, so the 
number of `availability zone`s per region is typically limited to 2~3.


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 #1397: Bookies should be from different racks in a Writequorum.

2018-05-09 Thread GitBox 
sijie commented on a change in pull request #1397: Bookies should be from 
different racks in a Writequorum.
URL: https://github.com/apache/bookkeeper/pull/1397#discussion_r187252145
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java
 ##
 @@ -525,17 +526,89 @@ public void testNewEnsembleWithMultipleRacks() throws 
Exception {
 addrs.add(addr4);
 repp.onClusterChanged(addrs, new HashSet());
 try {
-ArrayList ensemble = repp.newEnsemble(3, 2, 
2, null, new HashSet<>());
-int numCovered = getNumCoveredWriteQuorums(ensemble, 2);
+int ensembleSize = 3;
+int writeQuorumSize = 2;
+int acqQuorumSize = 2;
+ArrayList ensemble = 
repp.newEnsemble(ensembleSize, writeQuorumSize, acqQuorumSize,
+null, new HashSet<>());
+int numCovered = getNumCoveredWriteQuorums(ensemble, 2, 
numOfAvailableRacks);
 assertTrue(numCovered >= 1 && numCovered < 3);
-ArrayList ensemble2 = repp.newEnsemble(4, 2, 
2, null, new HashSet<>());
-numCovered = getNumCoveredWriteQuorums(ensemble2, 2);
+ensembleSize = 4;
+ArrayList ensemble2 = 
repp.newEnsemble(ensembleSize, writeQuorumSize, acqQuorumSize,
+null, new HashSet<>());
+numCovered = getNumCoveredWriteQuorums(ensemble2, 2, 
numOfAvailableRacks);
 assertTrue(numCovered >= 1 && numCovered < 3);
 } catch (BKNotEnoughBookiesException bnebe) {
 fail("Should not get not enough bookies exception even there is 
only one rack.");
 }
 }
 
+@Test
+public void testWriteQuorumNumberOfRacks() throws Exception {
+BookieSocketAddress addr1 = new BookieSocketAddress("127.0.0.2", 3181);
 
 Review comment:
   nit: can we put line 548-588 into a for-loop?


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 #1397: Bookies should be from different racks in a Writequorum.

2018-05-09 Thread GitBox 
sijie commented on a change in pull request #1397: Bookies should be from 
different racks in a Writequorum.
URL: https://github.com/apache/bookkeeper/pull/1397#discussion_r187252136
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicy.java
 ##
 @@ -525,17 +526,89 @@ public void testNewEnsembleWithMultipleRacks() throws 
Exception {
 addrs.add(addr4);
 repp.onClusterChanged(addrs, new HashSet());
 try {
-ArrayList ensemble = repp.newEnsemble(3, 2, 
2, null, new HashSet<>());
-int numCovered = getNumCoveredWriteQuorums(ensemble, 2);
+int ensembleSize = 3;
+int writeQuorumSize = 2;
+int acqQuorumSize = 2;
+ArrayList ensemble = 
repp.newEnsemble(ensembleSize, writeQuorumSize, acqQuorumSize,
+null, new HashSet<>());
+int numCovered = getNumCoveredWriteQuorums(ensemble, 2, 
numOfAvailableRacks);
 assertTrue(numCovered >= 1 && numCovered < 3);
-ArrayList ensemble2 = repp.newEnsemble(4, 2, 
2, null, new HashSet<>());
-numCovered = getNumCoveredWriteQuorums(ensemble2, 2);
+ensembleSize = 4;
+ArrayList ensemble2 = 
repp.newEnsemble(ensembleSize, writeQuorumSize, acqQuorumSize,
+null, new HashSet<>());
+numCovered = getNumCoveredWriteQuorums(ensemble2, 2, 
numOfAvailableRacks);
 assertTrue(numCovered >= 1 && numCovered < 3);
 } catch (BKNotEnoughBookiesException bnebe) {
 fail("Should not get not enough bookies exception even there is 
only one rack.");
 }
 }
 
+@Test
+public void testWriteQuorumNumberOfRacks() throws Exception {
 
 Review comment:
   can you add a unit test case to cover `write-quorum-size` is larger than 
`number-of-racks`?


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 #1395: ISSUE #1390 Ensemble change on delayed write error

2018-05-09 Thread GitBox 
sijie commented on issue #1395: ISSUE #1390 Ensemble change on delayed write 
error
URL: https://github.com/apache/bookkeeper/pull/1395#issuecomment-387967292
 
 
   retest this please


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