You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "bdeggleston (via GitHub)" <gi...@apache.org> on 2025/04/03 20:46:45 UTC
[PR] C20514 paxos ttl loop [cassandra]
bdeggleston opened a new pull request, #4043:
URL: https://github.com/apache/cassandra/pull/4043
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "pmcfadin (via GitHub)" <gi...@apache.org>.
pmcfadin closed pull request #4043: C20514 paxos ttl loop
URL: https://github.com/apache/cassandra/pull/4043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#discussion_r2027775778
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
Review Comment:
I wonder how you safely transition to a new Paxos state purging configuration? It's not atomic to change it across the entire cluster.
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
+ {
+ // In general, we need every replicas that have answered to the prepare (a quorum) to agree on the MRC (see
+ // coment in StorageProxy.beginAndRepairPaxos(), but basically we need to make sure at least a quorum of nodes
+ // have learn a commit before commit a new one otherwise that previous commit is not guaranteed to have reach a
+ // quorum and further commit may proceed on incomplete information).
+ // However, if that commit is too hold, it may have been expired from some of the replicas paxos table (we don't
Review Comment:
s/hold/old, s/replicas/replica's
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#discussion_r2027775778
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
Review Comment:
I wonder how you safely transition to a new Paxos state purging configuration? It's not atomic to change it across the entire cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#discussion_r2027769791
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
+ {
+ // In general, we need every replicas that have answered to the prepare (a quorum) to agree on the MRC (see
+ // coment in StorageProxy.beginAndRepairPaxos(), but basically we need to make sure at least a quorum of nodes
+ // have learn a commit before commit a new one otherwise that previous commit is not guaranteed to have reach a
+ // quorum and further commit may proceed on incomplete information).
+ // However, if that commit is too hold, it may have been expired from some of the replicas paxos table (we don't
Review Comment:
s/hold/old, s/replicas/replica's
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#discussion_r2027769791
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
+ {
+ // In general, we need every replicas that have answered to the prepare (a quorum) to agree on the MRC (see
+ // coment in StorageProxy.beginAndRepairPaxos(), but basically we need to make sure at least a quorum of nodes
+ // have learn a commit before commit a new one otherwise that previous commit is not guaranteed to have reach a
+ // quorum and further commit may proceed on incomplete information).
+ // However, if that commit is too hold, it may have been expired from some of the replicas paxos table (we don't
Review Comment:
s/hold/old
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#discussion_r2027770849
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
+ {
+ // In general, we need every replicas that have answered to the prepare (a quorum) to agree on the MRC (see
+ // coment in StorageProxy.beginAndRepairPaxos(), but basically we need to make sure at least a quorum of nodes
+ // have learn a commit before commit a new one otherwise that previous commit is not guaranteed to have reach a
+ // quorum and further commit may proceed on incomplete information).
+ // However, if that commit is too hold, it may have been expired from some of the replicas paxos table (we don't
+ // keep the paxos state forever or that could grow unchecked), and we could end up in some infinite loop as
+ // explained on CASSANDRA-12043. To avoid that, we ignore a MRC that is too old, i.e. older than the TTL we set
+ // on paxos tables. For such old commit, we rely on hints and repair to ensure the commit has indeed be
Review Comment:
```suggestion
// on paxos tables. For such an old commit, we rely on hints and repair to ensure the commit has indeed been
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "aweisberg (via GitHub)" <gi...@apache.org>.
aweisberg commented on code in PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#discussion_r2027770585
##########
src/java/org/apache/cassandra/service/paxos/v1/PrepareCallback.java:
##########
@@ -79,8 +86,28 @@ public synchronized void onResponse(Message<PrepareResponse> message)
latch.decrement();
}
- public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit()
+ public Iterable<InetAddressAndPort> replicasMissingMostRecentCommit(TableMetadata metadata)
{
+ /**
+ * this check is only needed for mixed mode operation with 4.0 and can be removed once upgrade support dropped
+ * see the comment in {@link org.apache.cassandra.distributed.upgrade.MixedModePaxosTTLTest} for a full explanation.
+ */
+ if (DatabaseDescriptor.paxosStatePurging() == Config.PaxosStatePurging.legacy)
+ {
+ // In general, we need every replicas that have answered to the prepare (a quorum) to agree on the MRC (see
+ // coment in StorageProxy.beginAndRepairPaxos(), but basically we need to make sure at least a quorum of nodes
+ // have learn a commit before commit a new one otherwise that previous commit is not guaranteed to have reach a
+ // quorum and further commit may proceed on incomplete information).
+ // However, if that commit is too hold, it may have been expired from some of the replicas paxos table (we don't
+ // keep the paxos state forever or that could grow unchecked), and we could end up in some infinite loop as
+ // explained on CASSANDRA-12043. To avoid that, we ignore a MRC that is too old, i.e. older than the TTL we set
Review Comment:
```suggestion
// explained on CASSANDRA-12043. To avoid that, we ignore an MRC that is too old, i.e. older than the TTL we set
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] C20514 paxos ttl loop [cassandra]
Posted by "pmcfadin (via GitHub)" <gi...@apache.org>.
pmcfadin commented on PR #4043:
URL: https://github.com/apache/cassandra/pull/4043#issuecomment-4227559062
Closing as this PR has had no activity for 1+ years and has no associated JIRA ticket. If this work is still relevant, please open a JIRA ticket and resubmit with a rebase against the current branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org