From 88ba1506af44d1e9f1252f155c27a1309607477d Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 24 May 2017 11:36:18 +0200 Subject: [PATCH] BUG-8371: Respond to CreateLocalHistoryRequest after replication CreateLocalHistoryRequest needs to be replicated to followers before we respond to the frontend, as logically this request has to be persisted before any subsequent transactions. While the frontend could replay the request on reconnect, it would also have to track the implied persistence (via child transactions), which we do not want because it really is a backend detail and it would lead to a lot of complexity in the frontend. Change-Id: Icdfad59d3c2bab3d4125186c6a9b3c901d3934f6 Signed-off-by: Robert Varga (cherry picked from commit 8f18717f60e58eebf726fe0611859311fa83df44) --- .../datastore/LeaderFrontendState.java | 31 ++++++++++++------- .../datastore/LocalFrontendHistory.java | 5 ++- .../cluster/datastore/ShardDataTree.java | 13 +++++--- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java index 8704f2ab0c..7e5addaefd 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java @@ -107,7 +107,7 @@ final class LeaderFrontendState implements Identifiable { try { if (request instanceof CreateLocalHistoryRequest) { - return handleCreateHistory((CreateLocalHistoryRequest) request); + return handleCreateHistory((CreateLocalHistoryRequest) request, envelope, now); } else if (request instanceof DestroyLocalHistoryRequest) { return handleDestroyHistory((DestroyLocalHistoryRequest) request, envelope, now); } else if (request instanceof PurgeLocalHistoryRequest) { @@ -121,30 +121,37 @@ final class LeaderFrontendState implements Identifiable { } } - private LocalHistorySuccess handleCreateHistory(final CreateLocalHistoryRequest request) throws RequestException { - final LocalHistoryIdentifier id = request.getTarget(); - final AbstractFrontendHistory existing = localHistories.get(id); + private LocalHistorySuccess handleCreateHistory(final CreateLocalHistoryRequest request, + final RequestEnvelope envelope, final long now) throws RequestException { + final LocalHistoryIdentifier historyId = request.getTarget(); + final AbstractFrontendHistory existing = localHistories.get(historyId); if (existing != null) { // History already exists: report success - LOG.debug("{}: history {} already exists", persistenceId, id); - return new LocalHistorySuccess(id, request.getSequence()); + LOG.debug("{}: history {} already exists", persistenceId, historyId); + return new LocalHistorySuccess(historyId, request.getSequence()); } // We have not found the history. Before we create it we need to check history ID sequencing so that we do not // end up resurrecting a purged history. - if (purgedHistories.contains(UnsignedLong.fromLongBits(id.getHistoryId()))) { + if (purgedHistories.contains(UnsignedLong.fromLongBits(historyId.getHistoryId()))) { LOG.debug("{}: rejecting purged request {}", persistenceId, request); throw new DeadHistoryException(purgedHistories); } // Update last history we have seen - if (lastSeenHistory == null || Long.compareUnsigned(lastSeenHistory, id.getHistoryId()) < 0) { - lastSeenHistory = id.getHistoryId(); + if (lastSeenHistory == null || Long.compareUnsigned(lastSeenHistory, historyId.getHistoryId()) < 0) { + lastSeenHistory = historyId.getHistoryId(); } - localHistories.put(id, LocalFrontendHistory.create(persistenceId, tree, id)); - LOG.debug("{}: created history {}", persistenceId, id); - return new LocalHistorySuccess(id, request.getSequence()); + // We have to send the response only after persistence has completed + final ShardDataTreeTransactionChain chain = tree.ensureTransactionChain(historyId, () -> { + LOG.debug("{}: persisted history {}", persistenceId, historyId); + envelope.sendSuccess(new LocalHistorySuccess(historyId, request.getSequence()), tree.ticker().read() - now); + }); + + localHistories.put(historyId, LocalFrontendHistory.create(persistenceId, tree, chain)); + LOG.debug("{}: created history {}", persistenceId, historyId); + return null; } private LocalHistorySuccess handleDestroyHistory(final DestroyLocalHistoryRequest request, diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalFrontendHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalFrontendHistory.java index 20d18e156e..d7c30121bf 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalFrontendHistory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LocalFrontendHistory.java @@ -36,9 +36,8 @@ final class LocalFrontendHistory extends AbstractFrontendHistory { } static LocalFrontendHistory create(final String persistenceId, final ShardDataTree tree, - final LocalHistoryIdentifier historyId) { - return new LocalFrontendHistory(persistenceId, tree, tree.ensureTransactionChain(historyId), ImmutableMap.of(), - TreeRangeSet.create()); + final ShardDataTreeTransactionChain chain) { + return new LocalFrontendHistory(persistenceId, tree, chain, ImmutableMap.of(), TreeRangeSet.create()); } static LocalFrontendHistory recreate(final String persistenceId, final ShardDataTree tree, diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java index 8f3a467d55..1aa180c51d 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java @@ -521,12 +521,15 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { return ret; } - ShardDataTreeTransactionChain ensureTransactionChain(final LocalHistoryIdentifier historyId) { + ShardDataTreeTransactionChain ensureTransactionChain(final LocalHistoryIdentifier historyId, + @Nullable final Runnable callback) { ShardDataTreeTransactionChain chain = transactionChains.get(historyId); if (chain == null) { chain = new ShardDataTreeTransactionChain(historyId, this); transactionChains.put(historyId, chain); - replicatePayload(historyId, CreateLocalHistoryPayload.create(historyId), null); + replicatePayload(historyId, CreateLocalHistoryPayload.create(historyId), callback); + } else if (callback != null) { + callback.run(); } return chain; @@ -537,7 +540,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { return new ReadOnlyShardDataTreeTransaction(this, txId, dataTree.takeSnapshot()); } - return ensureTransactionChain(txId.getHistoryId()).newReadOnlyTransaction(txId); + return ensureTransactionChain(txId.getHistoryId(), null).newReadOnlyTransaction(txId); } ReadWriteShardDataTreeTransaction newReadWriteTransaction(final TransactionIdentifier txId) { @@ -546,7 +549,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { .newModification()); } - return ensureTransactionChain(txId.getHistoryId()).newReadWriteTransaction(txId); + return ensureTransactionChain(txId.getHistoryId(), null).newReadWriteTransaction(txId); } @VisibleForTesting @@ -975,7 +978,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { return createReadyCohort(txId, mod); } - return ensureTransactionChain(txId.getHistoryId()).createReadyCohort(txId, mod); + return ensureTransactionChain(txId.getHistoryId(), null).createReadyCohort(txId, mod); } @SuppressFBWarnings(value = "DB_DUPLICATE_SWITCH_CLAUSES", justification = "See inline comments below.") -- 2.36.6