diff --git oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyDiff.java oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyDiff.java index 2ce59a13a3..e9fa6ce723 100644 --- oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyDiff.java +++ oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/standby/client/StandbyDiff.java @@ -63,7 +63,7 @@ class StandbyDiff implements NodeStateDiff { * missing binaries can be sync'ed if needed */ private final boolean logOnly; - + StandbyDiff(NodeBuilder builder, FileStore store, StandbyClient client, Supplier running) { this(builder, store, client, "/", false, running); } @@ -87,13 +87,11 @@ class StandbyDiff implements NodeStateDiff { if (stop()) { return false; } - - if (logOnly) { - binaryCheck(after); - } else { - builder.setProperty(binaryCheck(after)); + + if (!logOnly) { + builder.setProperty(after); } - + return true; } @@ -102,13 +100,11 @@ class StandbyDiff implements NodeStateDiff { if (stop()) { return false; } - - if (logOnly) { - binaryCheck(after); - } else { - builder.setProperty(binaryCheck(after)); + + if (!logOnly) { + builder.setProperty(after); } - + return true; } @@ -121,10 +117,70 @@ class StandbyDiff implements NodeStateDiff { if (!logOnly) { builder.removeProperty(before.getName()); } + + return true; + } + + @Override + public boolean childNodeAdded(String name, NodeState after) { + return process(name, "childNodeAdded", EmptyNodeState.EMPTY_NODE, after); + } + + @Override + public boolean childNodeChanged(String name, NodeState before, NodeState after) { + return process(name, "childNodeChanged", before, after); + } + + @Override + public boolean childNodeDeleted(String name, NodeState before) { + log.trace("childNodeDeleted {}, RO:{}", path + name, logOnly); + + if (!logOnly) { + builder.getChildNode(name).remove(); + } return true; } + private boolean process(String name, String op, NodeState before, NodeState after) { + if (stop()) { + return false; + } + + if (after instanceof SegmentNodeState) { + if (log.isTraceEnabled()) { + log.trace("{} {}, readonly binary check {}", op, path + name, logOnly); + } + + if (!logOnly) { + RecordId id = ((SegmentNodeState) after).getRecordId(); + builder.setChildNode(name, store.getReader().readNode(id)); + } + + if ("checkpoints".equals(name)) { + // if we're on the /checkpoints path, there's no need for a deep + // traversal to verify binaries + return true; + } + + if (!hasDataStore) { + return true; + } + + // has external data store, we need a deep + // traversal to verify binaries + + for (PropertyState propertyState : after.getProperties()) { + binaryCheck(propertyState); + } + + return after.compareAgainstBaseState(before, + new StandbyDiff(builder.getChildNode(name), store, client, path + name + "/", true, running)); + } + + return false; + } + private PropertyState binaryCheck(PropertyState property) { Type type = property.getType(); @@ -162,7 +218,7 @@ class StandbyDiff implements NodeStateDiff { } } } - + private void readBlob(String blobId, String pName) throws InterruptedException { InputStream in = client.getBlob(blobId); @@ -180,67 +236,4 @@ class StandbyDiff implements NodeStateDiff { } } - @Override - public boolean childNodeAdded(String name, NodeState after) { - return process(name, "childNodeAdded", EmptyNodeState.EMPTY_NODE, after); - } - - @Override - public boolean childNodeChanged(String name, NodeState before, NodeState after) { - try { - return process(name, "childNodeChanged", before, after); - } catch (RuntimeException e) { - log.trace("Check binaries for node {} and retry to process childNodeChanged", name); - // Attempt to load the binaries and retry, see OAK-4969 - for (PropertyState propertyState : after.getProperties()) { - binaryCheck(propertyState); - } - return process(name, "childNodeChanged", before, after); - } - } - - private boolean process(String name, String op, NodeState before, NodeState after) { - if (stop()) { - return false; - } - - if (after instanceof SegmentNodeState) { - if (log.isTraceEnabled()) { - log.trace("{} {}, readonly binary check {}", op, path + name, logOnly); - } - - if (!logOnly) { - RecordId id = ((SegmentNodeState) after).getRecordId(); - builder.setChildNode(name, store.getReader().readNode(id)); - } - - if ("checkpoints".equals(name)) { - // if we're on the /checkpoints path, there's no need for a deep - // traversal to verify binaries - return true; - } - - if (!hasDataStore) { - return true; - - } - // has external datastore, we need a deep - // traversal to verify binaries - return after.compareAgainstBaseState(before, new StandbyDiff(builder.getChildNode(name), store, client, path + name + "/", true, running)); - } - - return false; - } - - @Override - public boolean childNodeDeleted(String name, NodeState before) { - log.trace("childNodeDeleted {}, RO:{}", path + name, logOnly); - - if (!logOnly) { - builder.getChildNode(name).remove(); - } - - return true; - } - } diff --git oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/standby/ExternalPrivateStoreIT.java oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/standby/ExternalPrivateStoreIT.java index 304aed07c6..331c7c7708 100644 --- oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/standby/ExternalPrivateStoreIT.java +++ oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/standby/ExternalPrivateStoreIT.java @@ -18,12 +18,20 @@ */ package org.apache.jackrabbit.oak.segment.standby; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + import java.io.File; +import org.apache.jackrabbit.oak.segment.SegmentNodeStoreBuilders; import org.apache.jackrabbit.oak.segment.file.FileStore; +import org.apache.jackrabbit.oak.segment.standby.client.StandbyClientSync; +import org.apache.jackrabbit.oak.segment.standby.server.StandbyServerSync; import org.apache.jackrabbit.oak.segment.test.TemporaryBlobStore; import org.apache.jackrabbit.oak.segment.test.TemporaryFileStore; +import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.junit.Rule; +import org.junit.Test; import org.junit.rules.RuleChain; import org.junit.rules.TemporaryFolder; @@ -61,4 +69,23 @@ public class ExternalPrivateStoreIT extends DataStoreTestBase { return false; } + @Test + public void testSyncFailingDueToTooShortTimeout() throws Exception { + final int blobSize = 5 * MB; + FileStore primary = getPrimary(); + FileStore secondary = getSecondary(); + + NodeStore store = SegmentNodeStoreBuilders.builder(primary).build(); + addTestContent(store, "server", blobSize); + try ( + StandbyServerSync serverSync = new StandbyServerSync(serverPort.getPort(), primary, 1 * MB); + StandbyClientSync cl = newStandbyClientSync(secondary, serverPort.getPort(), false, 60) + ) { + serverSync.start(); + primary.flush(); + cl.run(); + assertNotEquals(primary.getHead(), secondary.getHead()); + assertEquals(1, cl.getFailedRequests()); + } + } }