CASSANDRA-19158 #3288
CASSANDRA-19158 #3288
Conversation
| } | ||
| else | ||
| { | ||
| throw new IllegalStateException(String.format("Queried for epoch %s, but could not catch up", awaitAtleast)); |
There was a problem hiding this comment.
do we want to throw here, or let the caller decide how to handle a failure to catch up? In some cases, this is purely a background task, so perhaps we should log but allow the caller to proceed? (I know we are certain that remote should be able to satisfy the request as we call this in response to seeing awaitAtLeast in some message from it).
There was a problem hiding this comment.
Right now, we will log this (rare) exception, which ji think is a reasonable behaviour.
On the other hand, if we have a listener, it will have to deal with exception one way or the other: this seems ok, too.
| return fetchLogAndWaitInternal(remoteRequest, candidateIterator, log); | ||
| try | ||
| { | ||
| return fetchLogAndWaitInternal(candidateIterator, log).awaitUninterruptibly().get(); |
There was a problem hiding this comment.
Should we awaitThrowUncheckedOnInterrupt, or even just await here? I'm thinking of tests really where the origin implementation (before CASSANDRA-19514) could cause occasional timeouts.
There was a problem hiding this comment.
Do you mean await with a timeout, or without? My logic for doing it this way was that we will rely on messaging service timeouts now to time this one out, so it should never be indefinite.
There was a problem hiding this comment.
fair enough, I was concerned about in-flight requests preventing a timely shutdown, but this will now be bounded by new Retry.Backoff(TCMMetrics.instance.fetchLogRetries));, right?
ifesdjeen
force-pushed
the
CASSANDRA-19158
branch
from
835ea6a
to
1d4e641
Compare
This reverts commit 90ff2b2.
ifesdjeen
force-pushed
the
CASSANDRA-19158
branch
from
1d4e641
to
7d7cde4
Compare
No description provided.