ivankelly commented on a change in pull request #1580: (@bug W-5100764@) Write
Ledger Handle listens for metadata changes
URL: https://github.com/apache/bookkeeper/pull/1580#discussion_r207493844
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
##########
@@ -2382,4 +2387,60 @@ public void closeComplete(int rc, LedgerHandle lh,
Object ctx) {
}
}
+ class MetadataMerger extends SafeRunnable {
+
+ final LedgerMetadata m;
+
+ MetadataMerger(LedgerMetadata metadata) {
+ this.m = metadata;
+ }
+
+ @Override
+ public void safeRun() {
+ Version.Occurred occurred =
+
LedgerHandle.this.metadata.getVersion().compare(this.m.getVersion());
+ if (Version.Occurred.BEFORE == occurred) {
+ LOG.info("Updated ledger metadata for ledger {} to {}.",
ledgerId, this.m.toSafeString());
+ synchronized (LedgerHandle.this) {
+ LedgerHandle.this.metadata.setVersion(m.getVersion());
+
LedgerHandle.this.metadata.mergeEnsembles(m.getEnsembles());
+ }
+ }
+ }
+
+ @Override
+ public String toString() {
+ return String.format("MetadataMerger(%d)", ledgerId);
+ }
+ }
+
+ @Override
+ public void onChanged(long ledgerId, LedgerMetadata newMetadata) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Received ledger metadata update on writeHandle {} : {}",
+ ledgerId, newMetadata);
+ }
+ if (this.ledgerId != ledgerId) {
+ // Ignore
+ return;
+ }
+ if (null == newMetadata) {
+ // Ignore
+ return;
+ }
+ Version.Occurred occurred =
+ this.metadata.getVersion().compare(newMetadata.getVersion());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Try to merge metadata from {} to {} : {}",
+ this.metadata, newMetadata, occurred);
+ }
+ if (Version.Occurred.BEFORE == occurred) { // the metadata is updated
+ try {
+ bk.getMainWorkerPool().executeOrdered(ledgerId, new
MetadataMerger(newMetadata));
Review comment:
There's no need to merge. The metadata read from zookeeper must have the
same last ensemble as the metadata currently being used, or else we're
violating a whole load of properties. So in theory, you should be able to
assign newMetadata to metadata. In practice it can be tricky with all the
mutation that occurs while handling bookie failure. So leave the merge for now,
but I will remove it once the ledger immutable metadata changes are in (I
should have remaining patches up today or monday/tuesday next week)
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
