date:20171019

[GitHub] sijie commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

sijie commented on a change in pull request #642: BP-14 part 1 - metadata and 
protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145789040
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/LedgerType.java
 ##
 @@ -0,0 +1,46 @@
+/**
+ *
+ * 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.api;
+
+/**
+ * Describes the type of ledger.
+ * LedgerTypes describes the behaviour of the ledger in respect to durability 
and provides
+ * hints to the storage of data on Bookies
+ *
+ * @since 4.6
+ */
+public enum LedgerType {
+/**
+ * Persistent Durability, using Journal.
+ * Each entry is persisted to the journal and every writes receives and 
acknowledgement only with the guarantee that
+ * it has been persisted durabily to it (data is fsync'd to the disk)
+ */
+PD_JOURNAL,
 
 Review comment:
   I am fine with REQUIRED and DEFERRED.


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] asfgit commented on issue #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

asfgit commented on issue #643: BP-14 part 2 - client side changes
URL: https://github.com/apache/bookkeeper/pull/643#issuecomment-337963302
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/124/
   


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] asfgit commented on issue #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

asfgit commented on issue #643: BP-14 part 2 - client side changes
URL: https://github.com/apache/bookkeeper/pull/643#issuecomment-337909287
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/122/
   


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] asfgit commented on issue #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

asfgit commented on issue #643: BP-14 part 2 - client side changes
URL: https://github.com/apache/bookkeeper/pull/643#issuecomment-337907689
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/121/
   


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-master #1922

2017-10-19 Thread Apache Jenkins Server

See 


Changes:

[ivan] Fixed ref counting release on read errors

[ivan] Fix concurrent v2 reads on the same ledger/entry

--
[...truncated 40.86 KB...]
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 36.302 sec - in 
org.apache.bookkeeper.client.UpdateLedgerOpTest
Running org.apache.bookkeeper.client.TestDelayEnsembleChange
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 8.062 sec - in 
org.apache.bookkeeper.client.TestDelayEnsembleChange
Running org.apache.bookkeeper.client.TestWeightedRandomSelection
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.399 sec - in 
org.apache.bookkeeper.client.TestWeightedRandomSelection
Running org.apache.bookkeeper.client.api.BookKeeperBuildersTest
Tests run: 24, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.9 sec - in 
org.apache.bookkeeper.client.api.BookKeeperBuildersTest
Running org.apache.bookkeeper.client.api.BookKeeperApiTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.789 sec - in 
org.apache.bookkeeper.client.api.BookKeeperApiTest
Running org.apache.bookkeeper.client.TestBookieWatcher
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.43 sec - in 
org.apache.bookkeeper.client.TestBookieWatcher
Running org.apache.bookkeeper.client.ParallelLedgerRecoveryTest
Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 39.467 sec - 
in org.apache.bookkeeper.client.ParallelLedgerRecoveryTest
Running org.apache.bookkeeper.client.BookieWriteLedgerTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 31.966 sec - 
in org.apache.bookkeeper.client.BookieWriteLedgerTest
Running org.apache.bookkeeper.client.GenericEnsemblePlacementPolicyTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.376 sec - in 
org.apache.bookkeeper.client.GenericEnsemblePlacementPolicyTest
Running org.apache.bookkeeper.client.TestAddEntryQuorumTimeout
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 10.822 sec - in 
org.apache.bookkeeper.client.TestAddEntryQuorumTimeout
Running org.apache.bookkeeper.client.BookKeeperCloseTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.491 sec - 
in org.apache.bookkeeper.client.BookKeeperCloseTest
Running 
org.apache.bookkeeper.client.TestRackawareEnsemblePlacementPolicyUsingScript
Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.589 sec - in 
org.apache.bookkeeper.client.TestRackawareEnsemblePlacementPolicyUsingScript
Running org.apache.bookkeeper.client.TestWatchEnsembleChange
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 28.208 sec - in 
org.apache.bookkeeper.client.TestWatchEnsembleChange
Running org.apache.bookkeeper.client.TestReadEntryListener
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.048 sec - in 
org.apache.bookkeeper.client.TestReadEntryListener
Running org.apache.bookkeeper.client.BookieWriteLedgersWithDifferentDigestsTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.865 sec - in 
org.apache.bookkeeper.client.BookieWriteLedgersWithDifferentDigestsTest
Running org.apache.bookkeeper.client.TestLedgerChecker
Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 10.683 sec - 
in org.apache.bookkeeper.client.TestLedgerChecker
Running org.apache.bookkeeper.client.TestPiggybackLAC
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.259 sec - in 
org.apache.bookkeeper.client.TestPiggybackLAC
Running org.apache.bookkeeper.client.TestDisableEnsembleChange
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 22.144 sec - in 
org.apache.bookkeeper.client.TestDisableEnsembleChange
Running org.apache.bookkeeper.client.ListLedgersTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.24 sec - in 
org.apache.bookkeeper.client.ListLedgersTest
Running org.apache.bookkeeper.client.TestReadLastConfirmedAndEntry
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.07 sec - in 
org.apache.bookkeeper.client.TestReadLastConfirmedAndEntry
Running org.apache.bookkeeper.client.TestRackawareEnsemblePlacementPolicy
Tests run: 17, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 8.963 sec - in 
org.apache.bookkeeper.client.TestRackawareEnsemblePlacementPolicy
Running org.apache.bookkeeper.client.TestRegionAwareEnsemblePlacementPolicy
Tests run: 30, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.852 sec - in 
org.apache.bookkeeper.client.TestRegionAwareEnsemblePlacementPolicy
Running org.apache.bookkeeper.client.SlowBookieTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 26.621 sec - in 
org.apache.bookkeeper.client.SlowBookieTest
Running org.apache.bookkeeper.client.BookKeeperTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.672 sec - 
in org.apache.bookkeeper.client.BookKeeperTest
Running

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145695709
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/SyncSupported.java
 ##
 @@ -0,0 +1,54 @@
+/**
+ *
+ * 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.api;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.nio.ByteBuffer;
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * Marks Handles which support the 'sync' primitive
+ *
+ * @see WriteHandle
+ * @see WriteAdvHandle
+ *
+ * @since 4.6
+ */
+public interface SyncSupported {
+
+/**
+ * Waits for all data written by this Handle to have been persisted 
durably on a quorum of bookies and advances
 
 Review comment:
   Yes,  WriteAdvHandle is tricky. You can leave gaps, then call sync() and 
then write entries in the gap.
   In this case sync() can only guarantee that data sent to bookies "before 
calling sync" has been persisted.
   Anyway the comment is right: "all data written by this Handle" are 
guaranteed to have been fsynched to the journal. And the returned value has no 
real meaning.
   Maybe it is  better to have two separate methods, and for WriteAdvHandle 
return a CompletableFuture


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 #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145693862
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RoundRobinDistributionSchedule.java
 ##
 @@ -138,4 +116,64 @@ public QuorumCoverageSet getCoverageSet() {
 public boolean hasEntry(long entryId, int bookieIndex) {
 return getWriteSet(entryId).contains(bookieIndex);
 }
+
+private class AckSetImpl implements AckSet {
 
 Review comment:
   mmm, I think you suggestion is good.
   I will try to move this logic into LedgerHandle and not per entry


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 #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145691150
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
 ##
 @@ -362,6 +402,14 @@ public synchronized boolean isClosed() {
 return metadata.isClosed();
 }
 
+/**
+ * Is ensemble change supported ?
+ * @return
+ */
+boolean isDelayEnsembleChangeSupported() {
 
 Review comment:
   this method is not even used. dropping it. Thank you @sijie . 
   
   I don't know why I had this in code. I am sorry I would not like to add 
noise to this complex work


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 #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145690148
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
 ##
 @@ -84,15 +85,32 @@
 final BookKeeper bk;
 final long ledgerId;
 long lastAddPushed;
+/**
+  * Last entryId which has been confirmed to be written durably to the 
bookies.
+  * This value is used by readers, the the LAC protocol
+  */
 volatile long lastAddConfirmed;
 
+/**
+  * Last entryId which has been confirmed to be written durably to the 
bookies,
+  * this is an internal variable used to track the status of entries and 
to handle correcly the lastAddConfirmed
+  * value
+  */
+ volatile long lastAddSynced;
 
 Review comment:
   you are right!  they contain the same value.
   I used that variable in my first prototype.
   Thank you very much @sijie 


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 #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145684521
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperClientStats.java
 ##
 @@ -55,13 +56,15 @@
 
 public final static String CHANNEL_READ_OP = "READ_ENTRY";
 public final static String CHANNEL_TIMEOUT_READ = "TIMEOUT_READ_ENTRY";
+public final static String CHANNEL_SYNC = "CHANNEL_SYNC";
 
 Review comment:
   done, renamed to "SYNC", is is not a problem that is it the same name as 
SYNC_OP?


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 #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145684461
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperClientStats.java
 ##
 @@ -32,6 +32,7 @@
 public final static String CREATE_OP = "LEDGER_CREATE";
 public final static String DELETE_OP = "LEDGER_DELETE";
 public final static String OPEN_OP = "LEDGER_OPEN";
+public final static String SYNC_OP = "LEDGER_SYNC";
 
 Review comment:
   done, renamed to "SYNC"


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 #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145684060
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java
 ##
 @@ -160,6 +160,25 @@ void readComplete(int rc, LedgerHandle lh, 
Enumeration seq,
 }
 
 /**
+ * Async Callback for handling sync responses
+ *
+ * @since 4.0
+ */
+@InterfaceAudience.Private
 
 Review comment:
   it was because I did not want to mark this as "internal use only", anyway I 
have just dropped 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] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #643: BP-14 part 2 - client 
side changes
URL: https://github.com/apache/bookkeeper/pull/643#discussion_r145684063
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java
 ##
 @@ -160,6 +160,25 @@ void readComplete(int rc, LedgerHandle lh, 
Enumeration seq,
 }
 
 /**
+ * Async Callback for handling sync responses
+ *
+ * @since 4.0
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Stable
+interface SyncCallback {
 
 Review comment:
   dropped


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145680332
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -193,3 +212,9 @@ message GetBookieInfoResponse {
 
 message StartTLSResponse {
 }
+
+message SyncResponse {
+required StatusCode status = 1;
+required int64 ledgerId = 2;
 
 Review comment:
   @ivankelly  Sorry, I can't understand your comment 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] asfgit commented on issue #643: BP-14 part 2 - client side changes

2017-10-19 Thread GitBox

asfgit commented on issue #643: BP-14 part 2 - client side changes
URL: https://github.com/apache/bookkeeper/pull/643#issuecomment-337872118
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/120/
   


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145631534
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##
 @@ -960,7 +961,8 @@ public void asyncCreateLedgerAdv(final int ensSize, final 
int writeQuorumSize, f
 return;
 }
 new LedgerCreateOp(BookKeeper.this, ensSize, writeQuorumSize,
-   ackQuorumSize, digestType, passwd, cb, ctx, 
customMetadata).initiateAdv((long)(-1));
+   ackQuorumSize, digestType, passwd, cb, ctx, 
customMetadata, LedgerType.PD_JOURNAL)
+.initiateAdv((long)(-1));
 
 Review comment:
   ok, but this is not really part of this change. I would like not to review 
all the usages of -1 and find which -1 are used for INVALID_LEDGER_ID
   This line has been changed only in order to add LedgerType.PD_JOURNAL
   
   I have created a BookieProtocol.INVALID_LEDGER_ID near INVALID_ENTRY_ID
   


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145640557
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -73,6 +74,14 @@ message BKPacketHeader {
 required uint64 txnId = 3;
 }
 
+/**
+ * Type of Ledger
+ */
+enum LedgerType {
 
 Review comment:
   I am changing this to a top level enum but I am hitting this bug on protoc
   https://github.com/google/protobuf/issues/2054


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145634367
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -136,6 +147,11 @@ message GetBookieInfoRequest {
 optional int64 requested = 1;
 }
 
+message SyncRequest {
+   required int64 ledgerId = 1;
 
 Review comment:
   because this API means 'force and wait for a sync on data for the given 
ledger"
   A bookie has several journals and we want to fsync data only on the journal 
assigned to the ledger (just for an example).
   
   If you do not have "journal" with this primitive the clients want the bookie 
to acknowledge that data sent to him for the ledger has been persisted durably


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145634439
 
 

 ##
 File path: bookkeeper-server/src/main/proto/DataFormats.proto
 ##
 @@ -58,6 +58,15 @@ message LedgerMetadataFormat {
 optional bytes value = 2;
 }
 repeated cMetadataMapEntry customMetadata = 11;
+
+/**
+* Ledger Type
+*/
+enum LedgerType {
 
 Review comment:
   I would like to use an integer or to use the same type


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145633907
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -83,6 +92,7 @@ message Request {
 optional ReadLacRequest readLacRequest = 104;
 optional GetBookieInfoRequest getBookieInfoRequest = 105;
 optional StartTLSRequest startTLSRequest = 106;
+optional SyncRequest syncRequest = 107;
 
 Review comment:
   This won't be used only for journalled bookies
   
@jvrao 


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145633790
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -62,6 +62,7 @@ enum OperationType {
 READ_LAC = 7;
 GET_BOOKIE_INFO = 8;
 START_TLS = 9;
+SYNC = 10;
 
 Review comment:
   But is maps to the 'sync()' function on the API


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145633637
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/LedgerType.java
 ##
 @@ -0,0 +1,46 @@
+/**
+ *
+ * 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.api;
+
+/**
+ * Describes the type of ledger.
+ * LedgerTypes describes the behaviour of the ledger in respect to durability 
and provides
+ * hints to the storage of data on Bookies
+ *
+ * @since 4.6
+ */
+public enum LedgerType {
+/**
+ * Persistent Durability, using Journal.
+ * Each entry is persisted to the journal and every writes receives and 
acknowledgement only with the guarantee that
+ * it has been persisted durabily to it (data is fsync'd to the disk)
+ */
+PD_JOURNAL,
 
 Review comment:
   The more I am working on this the more I am convinced that these names are 
not fine.
   
   We have to use the word 'journal" because @jvrao is working on a LedgerType 
without the Journal.
   And Volatile is the opposite of Durability
   
   Maybe
   SYNC_REQUIRED_WITH_JOURNAL
   SYNC_EXPLICIT_WITH_JOURNAL
   
   @sijie @jvrao  ?
   
   
   


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145632683
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -136,6 +147,11 @@ message GetBookieInfoRequest {
 optional int64 requested = 1;
 }
 
+message SyncRequest {
+   required int64 ledgerId = 1;
+   required bytes masterKey = 2;
 
 Review comment:
   you are right, it does not make sense.


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145632702
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -193,3 +212,9 @@ message GetBookieInfoResponse {
 
 message StartTLSResponse {
 }
+
+message SyncResponse {
+required StatusCode status = 1;
+required int64 ledgerId = 2;
+required int64 lastPersistedEntryId = 3;
 
 Review comment:
   will do


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145632181
 
 

 ##
 File path: bookkeeper-server/src/main/proto/BookkeeperProtocol.proto
 ##
 @@ -73,6 +74,14 @@ message BKPacketHeader {
 required uint64 txnId = 3;
 }
 
+/**
+ * Type of Ledger
+ */
+enum LedgerType {
 
 Review comment:
   Maybe the best option is to use an integer, it will be more simple to handle 
this in the future.
   @sijie  @ivankelly  What do you think ?
   


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145631841
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
 ##
 @@ -84,12 +85,14 @@
 
 private boolean hasPassword = false;
 private LedgerMetadataFormat.DigestType digestType;
+private LedgerMetadataFormat.LedgerType ledgerType;
 private byte[] password;
 
 private Map customMetadata = Maps.newHashMap();
 
 public LedgerMetadata(int ensembleSize, int writeQuorumSize, int 
ackQuorumSize,
 
 Review comment:
   @ivankelly  thank you I will let you drive this additional change


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 #642: BP-14 part 1 - metadata and protocol changes

2017-10-19 Thread GitBox

eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata 
and protocol changes
URL: https://github.com/apache/bookkeeper/pull/642#discussion_r145631534
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##
 @@ -960,7 +961,8 @@ public void asyncCreateLedgerAdv(final int ensSize, final 
int writeQuorumSize, f
 return;
 }
 new LedgerCreateOp(BookKeeper.this, ensSize, writeQuorumSize,
-   ackQuorumSize, digestType, passwd, cb, ctx, 
customMetadata).initiateAdv((long)(-1));
+   ackQuorumSize, digestType, passwd, cb, ctx, 
customMetadata, LedgerType.PD_JOURNAL)
+.initiateAdv((long)(-1));
 
 Review comment:
   ok, but this is not really part of this change. I would like not to review 
all the usages of -1 and find which -1 are used for INVALID_LEDGER_ID
   This line has been changed only in order to add LedgerType.PD_JOURNAL
   
   I will change to -1L
   


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 still unstable: bookkeeper-release-nightly-snapshot #79

2017-10-19 Thread Apache Jenkins Server

See

[GitHub] sijie commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] asfgit commented on issue #643: BP-14 part 2 - client side changes

[GitHub] asfgit commented on issue #643: BP-14 part 2 - client side changes

[GitHub] asfgit commented on issue #643: BP-14 part 2 - client side changes

Build failed in Jenkins: bookkeeper-master #1922

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] asfgit commented on issue #643: BP-14 part 2 - client side changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes

Jenkins build is still unstable: bookkeeper-release-nightly-snapshot #79

28 matches

Site Navigation

Mail list logo

Footer information