[GitHub] eolivelli commented on a change in pull request #1808: Allow to configure sticky reads
eolivelli commented on a change in pull request #1808: Allow to configure
sticky reads
URL: https://github.com/apache/bookkeeper/pull/1808#discussion_r246158954
##
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
##
@@ -1972,4 +2007,33 @@ public void closeComplete(int rc, LedgerHandle lh,
Object ctx) {
// becomes a property of the LedgerHandle itself.
return
LedgerMetadataUtils.getCurrentEnsemble(versionedMetadata.getValue());
}
+
+/**
+ * Return a {@link WriteSet} suitable for reading a particular entry.
+ * This will include all bookies that are cotna
+ */
+WriteSet getWriteSetForReadOperation(long entryId) {
+if (stickyBookieIndex != -1) {
+// When sticky reads are enabled we want to make sure to take
+// advantage of read-ahead (or, anyway, from efficiencies in
+// reading sequential data from disk through the page cache).
+// For this, all the entries that a given bookie prefetches,
+// should read from that bookie.
+// For example, with e=2, w=2, a=2 we would have
+// B-1 B-2
+// e-0 X X
+// e-1 X X
+// e-2 X X
+//
+// In this case we want all the requests to be issued to B-1 (by
+// preference), so that cache hits will be maximized.
+//
+// We can only enable sticky reads if the ensemble==writeQuorum
+// otherwise the same bookie will not have all the entries
+// stored
+return distributionSchedule.getWriteSet(stickyBookieIndex);
Review comment:
Okay. It works.
Sorry for late reply
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 #1808: Allow to configure sticky reads
eolivelli commented on a change in pull request #1808: Allow to configure
sticky reads
URL: https://github.com/apache/bookkeeper/pull/1808#discussion_r241944851
##
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
##
@@ -1972,4 +2007,33 @@ public void closeComplete(int rc, LedgerHandle lh,
Object ctx) {
// becomes a property of the LedgerHandle itself.
return
LedgerMetadataUtils.getCurrentEnsemble(versionedMetadata.getValue());
}
+
+/**
+ * Return a {@link WriteSet} suitable for reading a particular entry.
+ * This will include all bookies that are cotna
+ */
+WriteSet getWriteSetForReadOperation(long entryId) {
+if (stickyBookieIndex != -1) {
+// When sticky reads are enabled we want to make sure to take
+// advantage of read-ahead (or, anyway, from efficiencies in
+// reading sequential data from disk through the page cache).
+// For this, all the entries that a given bookie prefetches,
+// should read from that bookie.
+// For example, with e=2, w=2, a=2 we would have
+// B-1 B-2
+// e-0 X X
+// e-1 X X
+// e-2 X X
+//
+// In this case we want all the requests to be issued to B-1 (by
+// preference), so that cache hits will be maximized.
+//
+// We can only enable sticky reads if the ensemble==writeQuorum
+// otherwise the same bookie will not have all the entries
+// stored
+return distributionSchedule.getWriteSet(stickyBookieIndex);
Review comment:
Is this an entryId or a reference to a bookie.
I am a bit confused
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 #1808: Allow to configure sticky reads
eolivelli commented on a change in pull request #1808: Allow to configure
sticky reads
URL: https://github.com/apache/bookkeeper/pull/1808#discussion_r233328904
##
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java
##
@@ -96,13 +96,38 @@
this.ensemble = ensemble;
this.eId = eId;
+long entryIdToConsiderForWriteSet = eId;
+
+if (clientCtx.getConf().enableStickyReads
+&& lh.getLedgerMetadata().getWriteQuorumSize() ==
lh.getLedgerMetadata().getEnsembleSize()) {
+// When sticky reads are enabled we want to make sure to take
+// advantage of read-ahead (or, anyway, from effciencies in
+// reading sequential data from disk through the page cache).
+// For this, all the entries that a given bookie prefetches,
+// should read from that bookie.
+// For example, with e=2, w=2, a=2 we would have
+// B-1 B-2
+// e-0 X X
+// e-1 X X
+// e-2 X X
+//
+// In this case we want all the requests to be issued to B-1
(by
+// preference), so that cache hits will be maximized.
+//
+// We can only enable sticky reads if the ensemble==writeQuorum
+// otherwise the same bookie will not have all the entries
+// stored
+entryIdToConsiderForWriteSet = 0;
Review comment:
Okay
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] eolivelli commented on a change in pull request #1808: Allow to configure sticky reads
eolivelli commented on a change in pull request #1808: Allow to configure
sticky reads
URL: https://github.com/apache/bookkeeper/pull/1808#discussion_r233104196
##
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java
##
@@ -96,13 +96,38 @@
this.ensemble = ensemble;
this.eId = eId;
+long entryIdToConsiderForWriteSet = eId;
+
+if (clientCtx.getConf().enableStickyReads
+&& lh.getLedgerMetadata().getWriteQuorumSize() ==
lh.getLedgerMetadata().getEnsembleSize()) {
+// When sticky reads are enabled we want to make sure to take
+// advantage of read-ahead (or, anyway, from effciencies in
+// reading sequential data from disk through the page cache).
+// For this, all the entries that a given bookie prefetches,
+// should read from that bookie.
+// For example, with e=2, w=2, a=2 we would have
+// B-1 B-2
+// e-0 X X
+// e-1 X X
+// e-2 X X
+//
+// In this case we want all the requests to be issued to B-1
(by
+// preference), so that cache hits will be maximized.
+//
+// We can only enable sticky reads if the ensemble==writeQuorum
+// otherwise the same bookie will not have all the entries
+// stored
+entryIdToConsiderForWriteSet = 0;
Review comment:
Thinking out loud
Maybe the PlacementPolicy should decide about which bookie is to be used for
the 'sticky read'
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 #1808: Allow to configure sticky reads
eolivelli commented on a change in pull request #1808: Allow to configure
sticky reads
URL: https://github.com/apache/bookkeeper/pull/1808#discussion_r232907899
##
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java
##
@@ -96,13 +96,38 @@
this.ensemble = ensemble;
this.eId = eId;
+long entryIdToConsiderForWriteSet = eId;
+
+if (clientCtx.getConf().enableStickyReads
+&& lh.getLedgerMetadata().getWriteQuorumSize() ==
lh.getLedgerMetadata().getEnsembleSize()) {
+// When sticky reads are enabled we want to make sure to take
+// advantage of read-ahead (or, anyway, from effciencies in
+// reading sequential data from disk through the page cache).
+// For this, all the entries that a given bookie prefetches,
+// should read from that bookie.
+// For example, with e=2, w=2, a=2 we would have
+// B-1 B-2
+// e-0 X X
+// e-1 X X
+// e-2 X X
+//
+// In this case we want all the requests to be issued to B-1
(by
+// preference), so that cache hits will be maximized.
+//
+// We can only enable sticky reads if the ensemble==writeQuorum
+// otherwise the same bookie will not have all the entries
+// stored
+entryIdToConsiderForWriteSet = 0;
Review comment:
How do you deal with ensemble changes?
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
