You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "belliottsmith (via GitHub)" <gi...@apache.org> on 2026/06/08 11:21:50 UTC

[PR] CommandChanges.shouldCleanup short-circuits to NO if there is no data… [cassandra]

belliottsmith opened a new pull request, #4873:
URL: https://github.com/apache/cassandra/pull/4873

   …, but this is incorrect as Cleanup.EXPUNGE may have dropped the data and the record must receive cleanup EXPUNGE and be reported as ERASED. This can lead to lienarizability violations.
   
   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] CommandChanges.shouldCleanup short-circuits to NO if there is no data… [cassandra]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4873:
URL: https://github.com/apache/cassandra/pull/4873#discussion_r3396215297


##########
test/unit/org/apache/cassandra/service/accord/AccordExpungeTest.java:
##########
@@ -0,0 +1,211 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.service.accord;
+
+import java.nio.file.Files;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.assertj.core.api.SoftAssertions;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import accord.api.Journal;
+import accord.local.Cleanup;
+import accord.local.Command;
+import accord.local.DurableBefore;
+import accord.local.RedundantBefore;
+import accord.primitives.Ranges;
+import accord.primitives.Routable;
+import accord.primitives.SaveStatus;
+import accord.primitives.Timestamp;
+import accord.primitives.Txn;
+import accord.primitives.TxnId;
+import accord.utils.AccordGens;
+import accord.utils.Gen;
+import accord.utils.Gens;
+import accord.utils.TriConsumer;
+
+import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.io.util.File;
+import org.apache.cassandra.journal.TestParams;
+import org.apache.cassandra.schema.KeyspaceParams;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.service.accord.journal.AccordJournal;
+import org.apache.cassandra.service.accord.journal.CommandChanges;
+import org.apache.cassandra.utils.AccordGenerators;
+import org.apache.cassandra.utils.AccordGenerators.CommandBuilder;
+
+import static accord.local.Cleanup.Input.FULL;
+import static accord.local.RedundantStatus.SomeStatus.GC_BEFORE_AND_LOCALLY_DURABLE;
+import static accord.primitives.Routable.Domain.Range;
+import static accord.primitives.Txn.Kind.ExclusiveSyncPoint;
+import static accord.utils.Property.qt;
+import static org.apache.cassandra.cql3.statements.schema.CreateTableStatement.parse;
+
+/**
+ * Regression tests for the interaction between Accord journal cleanup decisions
+ * (driven by RedundantBefore / DurableBefore) and the {@code loadCommand} path.
+ *
+ * <p>Symptom: a transaction that has been processed and erased (or that was past the
+ * GC boundary by the time the journal saw it) is read back as {@code NotDefined}
+ * instead of {@code Erased}/{@code Truncated}.</p>
+ *
+ * <p>Mechanism under test: when the FULL-input cleanup path decides EXPUNGE,
+ * {@link accord.impl.CommandChange.Builder#construct} returns {@code null}.
+ * Downstream, {@link AccordSafeCommand#preExecute} maps {@code null} to a
+ * {@code Command.NotDefined.uninitialised(...)} — which is exactly the bogus
+ * NotDefined the user observed. The journal load API itself should never collapse
+ * "this txnId has been erased" into the same answer as "we have never heard of this
+ * txnId" once RedundantBefore / DurableBefore say it is past the GC boundary.</p>
+ *
+ * @author Claude and Benedict
+ */
+public class AccordExpungeTest
+{
+    private static final int COMMAND_STORE_ID = 1;
+
+    private final AtomicInteger counter = new AtomicInteger();
+
+    @BeforeClass
+    public static void setUpClass() throws Throwable
+    {
+        SchemaLoader.prepareServer();
+        // a single keyspace + table is enough; AccordGenerators.commandsBuilder() will use
+        // ks.tbl as the table for the synthetic Txn it produces.
+        SchemaLoader.createKeyspace("ks", KeyspaceParams.simple(1),
+                                    parse("CREATE TABLE tbl (k int, c int, v int, primary key (k, c)) WITH transactional_mode='full'", "ks"));
+        StorageService.instance.initServer();
+    }
+
+    @Before
+    public void beforeTest() throws Throwable
+    {
+        File directory = new File(Files.createTempDirectory(Integer.toString(counter.incrementAndGet())));
+        directory.deleteRecursiveOnExit();
+        DatabaseDescriptor.setAccordJournalDirectory(directory.path());
+    }
+
+    private void validate(TriConsumer<Command, RedundantBefore, DurableBefore> validate)
+    {
+        Gen<SaveStatus> saveStatusGen = Gens.enums().all(SaveStatus.class);
+        qt().forAll(commands().map((rs, b) -> b.build(saveStatusGen.next(rs))).filter(c -> !c.participants.touches().isEmpty()))
+            .check(before ->
+                   {
+                       Ranges ranges = before.participants.touches().toRanges();
+                       TxnId gcBound = gcBoundStrictlyAfter(before);
+                       RedundantBefore rb = RedundantBefore.create(ranges, Long.MIN_VALUE, Long.MAX_VALUE, gcBound,
+                                                                   GC_BEFORE_AND_LOCALLY_DURABLE);
+                       DurableBefore db = DurableBefore.create(ranges, gcBound, gcBound);
+
+                       Cleanup decided = Cleanup.shouldCleanup(FULL,
+                                                               before.txnId(),
+                                                               before.executeAt(),
+                                                               before.saveStatus(),
+                                                               before.durability(),
+                                                               before.participants(),
+                                                               rb, db);
+
+                       Assert.assertEquals(Cleanup.EXPUNGE, decided);
+
+                       validate.accept(before, rb, db);
+                   });
+    }
+
+    @Test
+    public void expungeReadsAsErased()
+    {
+        AccordJournal journal = newJournal();
+        try
+        {
+            journal.start(null);
+            validate((before, rb, db) -> {
+                CommandChanges builder = new CommandChanges(before.txnId);
+                builder.maybeCleanup(true, FULL, rb, db);
+                Command reconstructed = builder.construct(rb);
+                Assert.assertTrue("Empty builder for txnId=" + before.txnId()
+                                  + " constructed a non-Erased command despite being past GC: " + reconstructed,
+                                  reconstructed != null && reconstructed.saveStatus() == SaveStatus.Erased);
+
+                loadAndValidate(before, journal, rb, db);
+                journal.saveCommand(COMMAND_STORE_ID, new Journal.CommandUpdate(null, before), null);
+                loadAndValidate(before, journal, rb, db);
+                journal.closeCurrentSegmentForTestingIfNonEmpty();
+                loadAndValidate(before, journal, rb, db);
+            });
+        }
+        finally
+        {
+            journal.stop();
+        }
+    }
+
+    private static AccordJournal newJournal()
+    {
+        return new AccordJournal(new TestParams()
+        {
+            @Override public int segmentSize()        { return 1 << 20; }
+            @Override public boolean enableCompaction() { return false; }
+        });
+    }
+
+    private void loadAndValidate(Command before, Journal journal, RedundantBefore rb, DurableBefore db)
+    {
+        SoftAssertions checks = new SoftAssertions();
+        Command loaded = journal.loadCommand(COMMAND_STORE_ID, before.txnId, rb, db);
+        checks.assertThat(loaded).as("loadCommand returned null after RedundantBefore advance for %s; "
+                             + "AccordSafeCommand will surface this as NotDefined", loaded).isNotNull();
+
+        checks.assertThat(loaded.saveStatus())
+              .as("loadCommand did not return Erased for previously-written %s; "
+                  + "loaded=%s, redundantBefore=%s", before, loaded, rb)
+              .isEqualTo(SaveStatus.Erased);
+    }

Review Comment:
   `SoftAssertions` are created but never finalized with `assertAll()`, so failures may be silently ignored. Additionally, `loaded.saveStatus()` can throw NPE when `loaded` is null because SoftAssertions does not abort after `isNotNull()`.



##########
test/unit/org/apache/cassandra/service/accord/AccordExpungeTest.java:
##########
@@ -0,0 +1,211 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.service.accord;
+
+import java.nio.file.Files;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.assertj.core.api.SoftAssertions;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import accord.api.Journal;
+import accord.local.Cleanup;
+import accord.local.Command;
+import accord.local.DurableBefore;
+import accord.local.RedundantBefore;
+import accord.primitives.Ranges;
+import accord.primitives.Routable;
+import accord.primitives.SaveStatus;
+import accord.primitives.Timestamp;
+import accord.primitives.Txn;
+import accord.primitives.TxnId;
+import accord.utils.AccordGens;
+import accord.utils.Gen;
+import accord.utils.Gens;
+import accord.utils.TriConsumer;
+
+import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.io.util.File;
+import org.apache.cassandra.journal.TestParams;
+import org.apache.cassandra.schema.KeyspaceParams;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.service.accord.journal.AccordJournal;
+import org.apache.cassandra.service.accord.journal.CommandChanges;
+import org.apache.cassandra.utils.AccordGenerators;
+import org.apache.cassandra.utils.AccordGenerators.CommandBuilder;
+
+import static accord.local.Cleanup.Input.FULL;
+import static accord.local.RedundantStatus.SomeStatus.GC_BEFORE_AND_LOCALLY_DURABLE;
+import static accord.primitives.Routable.Domain.Range;
+import static accord.primitives.Txn.Kind.ExclusiveSyncPoint;
+import static accord.utils.Property.qt;
+import static org.apache.cassandra.cql3.statements.schema.CreateTableStatement.parse;
+
+/**
+ * Regression tests for the interaction between Accord journal cleanup decisions
+ * (driven by RedundantBefore / DurableBefore) and the {@code loadCommand} path.
+ *
+ * <p>Symptom: a transaction that has been processed and erased (or that was past the
+ * GC boundary by the time the journal saw it) is read back as {@code NotDefined}
+ * instead of {@code Erased}/{@code Truncated}.</p>
+ *
+ * <p>Mechanism under test: when the FULL-input cleanup path decides EXPUNGE,
+ * {@link accord.impl.CommandChange.Builder#construct} returns {@code null}.
+ * Downstream, {@link AccordSafeCommand#preExecute} maps {@code null} to a
+ * {@code Command.NotDefined.uninitialised(...)} — which is exactly the bogus
+ * NotDefined the user observed. The journal load API itself should never collapse
+ * "this txnId has been erased" into the same answer as "we have never heard of this
+ * txnId" once RedundantBefore / DurableBefore say it is past the GC boundary.</p>
+ *
+ * @author Claude and Benedict
+ */
+public class AccordExpungeTest
+{
+    private static final int COMMAND_STORE_ID = 1;
+
+    private final AtomicInteger counter = new AtomicInteger();
+
+    @BeforeClass
+    public static void setUpClass() throws Throwable
+    {
+        SchemaLoader.prepareServer();
+        // a single keyspace + table is enough; AccordGenerators.commandsBuilder() will use
+        // ks.tbl as the table for the synthetic Txn it produces.
+        SchemaLoader.createKeyspace("ks", KeyspaceParams.simple(1),
+                                    parse("CREATE TABLE tbl (k int, c int, v int, primary key (k, c)) WITH transactional_mode='full'", "ks"));
+        StorageService.instance.initServer();
+    }
+
+    @Before
+    public void beforeTest() throws Throwable
+    {
+        File directory = new File(Files.createTempDirectory(Integer.toString(counter.incrementAndGet())));
+        directory.deleteRecursiveOnExit();
+        DatabaseDescriptor.setAccordJournalDirectory(directory.path());
+    }
+
+    private void validate(TriConsumer<Command, RedundantBefore, DurableBefore> validate)
+    {
+        Gen<SaveStatus> saveStatusGen = Gens.enums().all(SaveStatus.class);
+        qt().forAll(commands().map((rs, b) -> b.build(saveStatusGen.next(rs))).filter(c -> !c.participants.touches().isEmpty()))
+            .check(before ->
+                   {
+                       Ranges ranges = before.participants.touches().toRanges();
+                       TxnId gcBound = gcBoundStrictlyAfter(before);
+                       RedundantBefore rb = RedundantBefore.create(ranges, Long.MIN_VALUE, Long.MAX_VALUE, gcBound,
+                                                                   GC_BEFORE_AND_LOCALLY_DURABLE);
+                       DurableBefore db = DurableBefore.create(ranges, gcBound, gcBound);
+
+                       Cleanup decided = Cleanup.shouldCleanup(FULL,
+                                                               before.txnId(),
+                                                               before.executeAt(),
+                                                               before.saveStatus(),
+                                                               before.durability(),
+                                                               before.participants(),
+                                                               rb, db);
+
+                       Assert.assertEquals(Cleanup.EXPUNGE, decided);
+
+                       validate.accept(before, rb, db);
+                   });
+    }
+
+    @Test
+    public void expungeReadsAsErased()
+    {
+        AccordJournal journal = newJournal();
+        try
+        {
+            journal.start(null);
+            validate((before, rb, db) -> {
+                CommandChanges builder = new CommandChanges(before.txnId);
+                builder.maybeCleanup(true, FULL, rb, db);
+                Command reconstructed = builder.construct(rb);

Review Comment:
   `CommandChanges` expects a `TxnId`, but this test passes `before.txnId` as if it were a field. Elsewhere in this file and in other Accord tests the `Command` txn id is accessed via `txnId()`; using field access here is inconsistent and may not compile if `txnId` is not a public member.



##########
test/distributed/org/apache/cassandra/service/accord/journal/AccordJournalBurnTest.java:
##########
@@ -101,6 +101,11 @@ public class AccordJournalBurnTest extends BurnTestBase
 {
     private static final Logger logger = LoggerFactory.getLogger(AccordJournalBurnTest.class);
 
+    static
+    {
+        Cluster.RandomLoader.CMD_BASE_CHECK_CHANCE = 0.1f;
+    }

Review Comment:
   Setting `Cluster.RandomLoader.CMD_BASE_CHECK_CHANCE` in a static initializer mutates global state for the whole JVM and is not restored, which can leak into other distributed tests depending on execution order. Prefer setting it in setup with a corresponding reset in teardown (e.g., `@BeforeClass/@AfterClass`) or scoping it to the test with a try/finally restore.



##########
.gitmodules:
##########
@@ -1,4 +1,4 @@
 [submodule "modules/accord"]
 	path = modules/accord
-	url = https://github.com/apache/cassandra-accord.git
-	branch = trunk
+	url = https://github.com/belliottsmith/cassandra-accord.git
+	branch = consistent-expunge

Review Comment:
   This changes the Accord submodule to point at a personal fork/branch. That makes the build depend on a non-canonical repository and makes the PR’s functional fix effectively unreviewable from this repo alone. The submodule should continue to track the canonical Apache repo/branch; if a specific fix is needed, update the submodule gitlink to the desired commit in the canonical repo (or land the fix upstream first).



-- 
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] CommandChanges.shouldCleanup short-circuits to NO if there is no data… [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #4873:
URL: https://github.com/apache/cassandra/pull/4873#discussion_r3420025520


##########
src/java/org/apache/cassandra/db/rows/BTreeRow.java:
##########
@@ -143,12 +143,17 @@ public static BTreeRow emptyRow(Clustering<?> clustering)
     }
 
     public static BTreeRow singleCellRow(Clustering<?> clustering, Cell<?> cell)
+    {
+        return singleCellRow(clustering, LivenessInfo.EMPTY, cell);
+    }
+
+    public static BTreeRow singleCellRow(Clustering<?> clustering, LivenessInfo primaryKeyLivenessInfo, Cell<?> cell)
     {
         if (cell.column().isSimple())
-            return new BTreeRow(clustering, BTree.singleton(cell), minDeletionTime(cell));
+            return new BTreeRow(clustering, primaryKeyLivenessInfo, Deletion.LIVE, BTree.singleton(cell), minDeletionTime(cell));

Review Comment:
   Good catch



-- 
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] CommandChanges.shouldCleanup short-circuits to NO if there is no data… [cassandra]

Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #4873:
URL: https://github.com/apache/cassandra/pull/4873#discussion_r3418748066


##########
src/java/org/apache/cassandra/db/rows/BTreeRow.java:
##########
@@ -143,12 +143,17 @@ public static BTreeRow emptyRow(Clustering<?> clustering)
     }
 
     public static BTreeRow singleCellRow(Clustering<?> clustering, Cell<?> cell)
+    {
+        return singleCellRow(clustering, LivenessInfo.EMPTY, cell);
+    }
+
+    public static BTreeRow singleCellRow(Clustering<?> clustering, LivenessInfo primaryKeyLivenessInfo, Cell<?> cell)
     {
         if (cell.column().isSimple())
-            return new BTreeRow(clustering, BTree.singleton(cell), minDeletionTime(cell));
+            return new BTreeRow(clustering, primaryKeyLivenessInfo, Deletion.LIVE, BTree.singleton(cell), minDeletionTime(cell));

Review Comment:
   Do we want to do 
   ```
   long minDeletionTime = Math.min(minDeletionTime(primaryKeyLivenessInfo), minDeletionTime(deletion.time()));
   ```
   
   as we do in `create` above here too?



##########
src/java/org/apache/cassandra/service/accord/AccordService.java:
##########
@@ -622,11 +622,24 @@ public synchronized void localStartup()
             {
                 replayJournal(minSegments);
 
-                logger.info("Try to execute pending transactions...");
-                List<AsyncResult<Void>> results = new ArrayList<>();
-                node.commandStores().forAllUnsafe(commandStore -> results.add(commandStore.tryToExecuteListeningTxns(false)));
-                if (!results.isEmpty())
-                    getBlocking(AsyncResults.reduce(results, Reduce.toNull()));
+                if (getAccord().execute_waiting_on_start)
+                {
+                    logger.info("Execute waiting transactions...");
+                    List<AsyncResult<Void>> results = new ArrayList<>();
+                    node.commandStores().forAllUnsafe(commandStore -> results.add(commandStore.tryToExecuteListeningTxns(false)));
+                    if (!results.isEmpty())
+                    {
+                        Future<?> future = toFuture(AsyncResults.reduce(results, Reduce.toNull()));
+                        long timeoutSeconds = getAccord().execute_waiting_on_start_timeout.toSeconds();
+                        if (timeoutSeconds <= 0) future.awaitUninterruptibly().rethrowIfFailed();
+                        else if (!future.awaitUninterruptibly(timeoutSeconds, SECONDS))

Review Comment:
   I think we would throw here only on timeout, but not on failure; should we add `future.isSuccess()` too?



-- 
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] CommandChanges.shouldCleanup short-circuits to NO if there is no data… [cassandra]

Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #4873:
URL: https://github.com/apache/cassandra/pull/4873#discussion_r3420015477


##########
src/java/org/apache/cassandra/service/accord/AccordService.java:
##########
@@ -622,11 +622,24 @@ public synchronized void localStartup()
             {
                 replayJournal(minSegments);
 
-                logger.info("Try to execute pending transactions...");
-                List<AsyncResult<Void>> results = new ArrayList<>();
-                node.commandStores().forAllUnsafe(commandStore -> results.add(commandStore.tryToExecuteListeningTxns(false)));
-                if (!results.isEmpty())
-                    getBlocking(AsyncResults.reduce(results, Reduce.toNull()));
+                if (getAccord().execute_waiting_on_start)
+                {
+                    logger.info("Execute waiting transactions...");
+                    List<AsyncResult<Void>> results = new ArrayList<>();
+                    node.commandStores().forAllUnsafe(commandStore -> results.add(commandStore.tryToExecuteListeningTxns(false)));
+                    if (!results.isEmpty())
+                    {
+                        Future<?> future = toFuture(AsyncResults.reduce(results, Reduce.toNull()));
+                        long timeoutSeconds = getAccord().execute_waiting_on_start_timeout.toSeconds();
+                        if (timeoutSeconds <= 0) future.awaitUninterruptibly().rethrowIfFailed();
+                        else if (!future.awaitUninterruptibly(timeoutSeconds, SECONDS))

Review Comment:
   Weird, I thought I already handled that. Thanks!



-- 
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] CommandChanges.shouldCleanup short-circuits to NO if there is no data… [cassandra]

Posted by "alanwang67 (via GitHub)" <gi...@apache.org>.
alanwang67 commented on PR #4873:
URL: https://github.com/apache/cassandra/pull/4873#issuecomment-4673906189

   +1


-- 
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