Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ce1071e
Fix bugs in fixLag()
DaveCTurner Oct 7, 2018
5c20e63
Add low-level bootstrap implementation
DaveCTurner Oct 7, 2018
cff9df7
Merge branch '2018-10-07-low-level-bootstrapping' into 2018-10-08-mer…
DaveCTurner Oct 8, 2018
d6d1ee4
Add storage-layer disruptions to CoordinatorTests
DaveCTurner Oct 8, 2018
5e7a4f9
Assert that we clean up disruptStorage correctly
DaveCTurner Oct 8, 2018
5c47f55
Better message in the case where a quorum has not been discovered
DaveCTurner Oct 8, 2018
dbb2b1a
Review feedback
DaveCTurner Oct 8, 2018
ef712a5
Set initial configuration at the start of stabilisation
DaveCTurner Oct 8, 2018
dcbacd8
Describe why the first election should succeed
DaveCTurner Oct 8, 2018
6644618
Merge branch '2018-10-07-low-level-bootstrapping' into 2018-10-08-dis…
DaveCTurner Oct 8, 2018
040657b
Merge branch 'zen2' into 2018-10-08-disrupt-storage
DaveCTurner Oct 8, 2018
b81ef53
Fail before or after
DaveCTurner Oct 11, 2018
9bd4741
Handle exception when bumping term
DaveCTurner Oct 11, 2018
1db8419
Throw more kinds of exception
DaveCTurner Oct 11, 2018
c72e863
Log too
DaveCTurner Oct 11, 2018
b66b991
Merge branch 'zen2' into 2018-10-08-disrupt-storage
DaveCTurner Oct 11, 2018
6ce5f67
More finallys
DaveCTurner Oct 12, 2018
68bdc08
More finally
DaveCTurner Oct 12, 2018
e1e33fa
Fix node to follow
DaveCTurner Oct 12, 2018
3dac8fa
become candidate on failure
DaveCTurner Oct 12, 2018
ac1d564
Do not always start election scheduler
DaveCTurner Oct 12, 2018
27de8d7
Revert "Do not always start election scheduler"
DaveCTurner Oct 13, 2018
7b6985b
Revert "become candidate on failure"
DaveCTurner Oct 13, 2018
02c1cf1
Revert "Fix node to follow"
DaveCTurner Oct 13, 2018
d43916e
Revert "More finally"
DaveCTurner Oct 13, 2018
22f5af7
Revert "More finallys"
DaveCTurner Oct 13, 2018
38fac65
Revert "Handle exception when bumping term"
DaveCTurner Oct 13, 2018
fce76c2
Remove possiblyFail() calls after updating state
DaveCTurner Oct 13, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ public static long value(ClusterState clusterState) {
}

public static class InMemoryPersistedState implements PersistedState {
// TODO add support and tests for behaviour with persistence-layer failures

private long currentTerm;
private ClusterState acceptedState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import org.hamcrest.Matcher;
import org.junit.Before;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -522,6 +524,7 @@ class Cluster {
final DeterministicTaskQueue deterministicTaskQueue = new DeterministicTaskQueue(
// TODO does ThreadPool need a node name any more?
Settings.builder().put(NODE_NAME_SETTING.getKey(), "deterministic-task-queue").build(), random());
private boolean disruptStorage;
private final VotingConfiguration initialConfiguration;

private final Set<String> disconnectedNodes = new HashSet<>();
Expand Down Expand Up @@ -566,6 +569,7 @@ void runRandomly() {
logger.info("--> start of safety phase of at least [{}] steps", randomSteps);

deterministicTaskQueue.setExecutionDelayVariabilityMillis(EXTREME_DELAY_VARIABILITY);
disruptStorage = true;
int step = 0;
long finishTime = -1;

Expand Down Expand Up @@ -636,7 +640,7 @@ void runRandomly() {
// - reboot a node
// - abdicate leadership

} catch (CoordinationStateRejectedException ignored) {
} catch (CoordinationStateRejectedException | UncheckedIOException ignored) {
// This is ok: it just means a message couldn't currently be handled.
}

Expand All @@ -645,6 +649,7 @@ void runRandomly() {

disconnectedNodes.clear();
blackholedNodes.clear();
disruptStorage = false;
}

private void assertConsistentStates() {
Expand Down Expand Up @@ -674,6 +679,7 @@ void stabilise() {
void stabilise(long stabilisationDurationMillis) {
assertThat("stabilisation requires default delay variability (and proper cleanup of raised variability)",
deterministicTaskQueue.getExecutionDelayVariabilityMillis(), lessThanOrEqualTo(DEFAULT_DELAY_VARIABILITY));
assertFalse("stabilisation requires stable storage", disruptStorage);

if (clusterNodes.stream().allMatch(n -> n.coordinator.getLastAcceptedState().getLastAcceptedConfiguration().isEmpty())) {
assertThat("setting initial configuration may fail with disconnected nodes", disconnectedNodes, empty());
Expand Down Expand Up @@ -826,6 +832,37 @@ ClusterNode getAnyNodePreferringLeaders() {
return getAnyNode();
}

class MockPersistedState extends InMemoryPersistedState {
MockPersistedState(long term, ClusterState acceptedState) {
super(term, acceptedState);
}

private void possiblyFail(String description) {
if (disruptStorage && rarely()) {
// TODO revisit this when we've decided how PersistedState should throw exceptions
if (randomBoolean()) {
throw new UncheckedIOException(new IOException("simulated IO exception [" + description + ']'));
} else {
throw new CoordinationStateRejectedException("simulated IO exception [" + description + ']');
}
}
}

@Override
public void setCurrentTerm(long currentTerm) {
possiblyFail("before writing term of " + currentTerm);
super.setCurrentTerm(currentTerm);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe inject failure both before or after executing the actual action.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I tried this and it's found a lot of places where we assume an exception means the write was unsuccessful. We can fix these, but I am not sure this is the right thing to do. We plan on finishing each write with a rename-and-fsync-the-directory. A failure before the fsync (including during the rename) means we didn't change state. I'm unsure how we should interpret a failed fsync.

/cc @andrershov re #33958

// TODO possiblyFail() here if that's a failure mode of the storage layer
}

@Override
public void setLastAcceptedState(ClusterState clusterState) {
possiblyFail("before writing last-accepted state of term=" + clusterState.term() + ", version=" + clusterState.version());
super.setLastAcceptedState(clusterState);
// TODO possiblyFail() here if that's a failure mode of the storage layer
}
}

class ClusterNode extends AbstractComponent {
private final int nodeIndex;
private Coordinator coordinator;
Expand All @@ -841,7 +878,7 @@ class ClusterNode extends AbstractComponent {
super(Settings.builder().put(NODE_NAME_SETTING.getKey(), nodeIdFromIndex(nodeIndex)).build());
this.nodeIndex = nodeIndex;
localNode = createDiscoveryNode();
persistedState = new InMemoryPersistedState(0L,
persistedState = new MockPersistedState(0L,
clusterState(0L, 0L, localNode, VotingConfiguration.EMPTY_CONFIG, VotingConfiguration.EMPTY_CONFIG, 0L));
onNode(localNode, this::setUp).run();
}
Expand Down