[CASSANDRA-20059] TCM's Retry.Deadline#retryIndefinitely is dangerous if used with RemoteProcessor as the deadline does not impact message retries

• Type: Bug
• Status: Resolved
• Priority: Normal
• Resolution: Fixed
• Fix Version/s: 5.1
• Component/s: Transactional Cluster Metadata
• Bug Category: Correctness - API / Semantic Implementation
• Severity: Normal
• Complexity: Low Hanging Fruit
• Discovered By: Code Inspection
• Platform: All
• Since Version: 5.1

Source Control Link

https://github.com/apache/cassandra/commit/4f49ca5e29d9c7207654a1f3c4eac9c9f0b84e5e

Test and Documentation Plan

existing tests

• Assignee: David Capwell
• Reporter: David Capwell

Authors

David Capwell

Reviewers

Alex Petrov, Sam Tunnicliffe

• Created: 06/Nov/24 19:05
• Updated: 10/Dec/24 23:28
• Resolved: 10/Dec/24 23:28

Description

public static Deadline retryIndefinitely(long timeoutNanos, Meter retryMeter)
{
    return new Deadline(Clock.Global.nanoTime() + timeoutNanos,
                        new Retry.Jitter(Integer.MAX_VALUE, DEFAULT_BACKOFF_MS, new Random(), retryMeter))
    {
        @Override
        public boolean reachedMax()
        {
            return false;
        }

        @Override
        public long remainingNanos()
        {
            return timeoutNanos;
        }

        public String toString()
        {
            return String.format("RetryIndefinitely{tries=%d}", currentTries());
        }
    };
}

Sample usage pattern (example is in Accord, but same pattern exists in RemoteProcessor.commit)

Promise<LogState> request = new AsyncPromise<>();
List<InetAddressAndPort> candidates = new ArrayList<>(log.metadata().fullCMSMembers());
sendWithCallbackAsync(request,
                      Verb.TCM_RECONSTRUCT_EPOCH_REQ,
                      new ReconstructLogState(lowEpoch, highEpoch, includeSnapshot),
                      new CandidateIterator(candidates),
                      retryPolicy);
return request.get(retryPolicy.remainingNanos(), TimeUnit.NANOSECONDS);

The issue here is that the networking retry has no clue that we gave up waiting on the request, so we will keep retrying until success! The reason for this is “reachedMax” is used to see if its safe to run again, but it isn’t as the deadline has passed!

Issue Links

• links to: GH: cep-15-accord
• links to: GitHub Pull Request #3670