[GitHub] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes
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_r146538874
##
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:
@ivankelly @sijie @jvrao
Proposal from Ivan sounds very good to me
FORCE_ON_JOURNAL (default)
FORCE_DEFERRED_ON_JOURNAL
SKIP_JOURNAL (this is a hint for the future, not part of BP-14)
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
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_r146520915
##
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:
If we rename 'sync' to force, for the LedgerType @sijie @ivankelly what is
your idea ?
FSYNC_REQUIRED_ON_JOURNAL vs FORCE_REQUIRED_ON_JOURNAL vs
AUTOMATIC_FORCE_ON_JOURNAL
and
FSYNC_DEFERRED_ON_JOURNAL vs FORCE_DEFERRED_ON_JOURNAL vs
EXPLICIT_FORCE_ON_JOURNAL
and SKIP_JOURNAL (@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
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_r146520352
##
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:
OK, I will rename Sync with Force, so ForceRequest, FORCE,
WriteHandle#force()
I will wait for some time before actually performing the rename, I am
preparing other patches and this will be very disruptive
@jvrao Are you OK with this new name 'force' instead of '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 #642: BP-14 part 1 - metadata and protocol changes
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_r146036457
##
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:
Anche for this @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 #642: BP-14 part 1 - metadata and protocol changes
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_r146480119
##
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:
This is an example:
```java
try (WriteHandle wh...) {
wh.append(data);
wh.append(data);
wh.sync();
wh.append(data);
}
```
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
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_r146037170
##
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:
Better 'with' as some ledger type won't have journal
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
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_r146036846
##
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:
Rollback. Thisi was needed, in order to access ledger impl on bookie side.
See third part
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
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_r146036543
##
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:
And for 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] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes
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_r146036457
##
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:
Anche for this @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 #642: BP-14 part 1 - metadata and protocol changes
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_r145971307
##
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:
sorry, @sijie
SYNC_REQUIRED_WITH_JOURNAL, SYNC_DEFERRED_WITH_JOURNAL
or
FSYNC_REQUIRED, FSYNC_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] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes
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] eolivelli commented on a change in pull request #642: BP-14 part 1 - metadata and protocol changes
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
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
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
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
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
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
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
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
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
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
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
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_r145631687
##
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java
##
@@ -218,6 +224,8 @@ private void createComplete(int rc, LedgerHandle lh) {
private byte[] builderPassword;
private org.apache.bookkeeper.client.api.DigestType builderDigestType
= org.apache.bookkeeper.client.api.DigestType.CRC32;
+private org.apache.bookkeeper.client.api.LedgerType builderLedgerType
Review comment:
yup, it was a cut and paste from the DigestType which needs the FQN
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
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
