-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cassandra 19719 #3382
Cassandra 19719 #3382
Conversation
@@ -304,7 +344,22 @@ public boolean invokeOnFailure() | |||
{ | |||
return true; | |||
} | |||
}); | |||
}; | |||
if (!failureDetector.isAlive(toEp)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly this will still log the error that the address isn't known...
{ | ||
// was the endpoint removed from membership? | ||
ClusterMetadata metadata = ClusterMetadata.current(); | ||
if (Gossiper.instance.getEndpointStateForEndpoint(toEp) == null && !metadata.directory.allJoinedEndpoints().contains(toEp) && !metadata.fullCMSMembers().contains(toEp)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was thinking it would be good to refactor as this was copy/pasted from FailureDetector
but wasn't sure where to put it...
import org.apache.cassandra.utils.concurrent.AsyncPromise; | ||
import org.apache.cassandra.utils.concurrent.Future; | ||
|
||
public class SimulatedMessageDelivery implements MessageDelivery |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was pulled out of the repair simulation tests
{ | ||
Ranges ranges = AccordTopology.createAccordTopology(current).ranges().mergeTouching(); | ||
if (!current.directory.isEmpty()) | ||
Assertions.assertThat(ranges).hasSize(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there was a test bug early on where there were gaps in the token ring... this isn't needed anymore, but left just to make sure nothing breaks again
@@ -1278,13 +1278,12 @@ | |||
<condition property="maxMemory" value="8G" else="4G"> | |||
<equals arg1="${test.classlistprefix}" arg2="distributed"/> | |||
</condition> | |||
<testmacrohelper inputdir="${test.dir}/${test.classlistprefix}" filelist="@{test.file.list}" exclude="**/*.java" timeout="${test.timeout}"> | |||
<testmacrohelper inputdir="${test.dir}/${test.classlistprefix}" filelist="@{test.file.list}" exclude="**/*.java" timeout="${test.timeout}" maxmemory="${maxMemory}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new epoch sync test was failing and turns out we try to set Xmx below, but that is override by testmacrohelper
who uses 1024m
. Need to pass maxMemory
along to avoid that...
there is a thread about this in https://the-asf.slack.com/archives/CK23JSY2K/p1718991597495909
Accord branch has the maxmemory
as a property we can override, but not on any other branch...
|
||
public class SimulatedMessageDelivery implements MessageDelivery | ||
{ | ||
public enum Action { DELIVER, DELIVER_WITH_FAILURE, DROP, DROP_PARTITIONED, FAILURE } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added more actions than repair normally supports, this is more in-line with BurnTests' NodeSink
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some minor change, looks good otherwise
src/java/org/apache/cassandra/service/accord/AccordConfigurationService.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/service/accord/AccordSyncPropagator.java
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/service/accord/AccordSyncPropagator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
…e topology tracking to avoid being blocked patch by David Capwell; reviewed by Blake Eggleston for CASSANDRA-19719
No description provided.