[
https://issues.apache.org/jira/browse/IGNITE-22279?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Vladislav Pyatkov updated IGNITE-22279:
---------------------------------------
Description:
https://github.com/apache/ignite-3/pull/3704
h3. Motivation
Unfortunately, we merged PR before all the comments were published. Except for
other formalities, there is one main note:
{code}
if (retryExecutor != null && matchAny(unwrapCause(errResp.throwable()),
ACQUIRE_LOCK_ERR, REPLICA_MISS_ERR)) {
retryExecutor.schedule(
// Need to resubmit again to pool which is valid for synchronous IO
execution.
() -> partitionOperationsExecutor.execute(() ->
res.completeExceptionally(errResp.throwable())),
RETRY_TIMEOUT_MILLIS, MILLISECONDS);
}
{code}
In the snipped, we put the response off in 20 milliseconds to retry the request
in when the perious cercumstances are changed. But here we delay of handling
the request intntinaly, that coud be a perfomance issue. Moreover, the delay
does not have to apply for the replica miss exception (this exception is
handled in client side through using the placement driver API).
h3. Definition of done
Excluded the handler for the replica miss exception and handled this exception
on the client side.
Add the logic of calculating the count of retries here, and do not delay
response when the retry is not assumed.
Move the timeout configuration (it cannot be a const) to the configuration
property in the same place where the configuration of dead lock prevention is
stored.
Look at all the other comments in the PR
(https://github.com/apache/ignite-3/pull/3704)
was:
https://github.com/apache/ignite-3/pull/3704
h3. Motivation
{code}
if (retryExecutor != null && matchAny(unwrapCause(errResp.throwable()),
ACQUIRE_LOCK_ERR, REPLICA_MISS_ERR)) {
retryExecutor.schedule(
// Need to resubmit again to pool which is
valid for synchronous IO execution.
() ->
partitionOperationsExecutor.execute(() ->
res.completeExceptionally(errResp.throwable())),
RETRY_TIMEOUT_MILLIS, MILLISECONDS);
}
{code}
> Missed comments in PR IGNITE-22130
> ----------------------------------
>
> Key: IGNITE-22279
> URL: https://issues.apache.org/jira/browse/IGNITE-22279
> Project: Ignite
> Issue Type: Bug
> Reporter: Vladislav Pyatkov
> Priority: Major
> Labels: ignite-3
>
> https://github.com/apache/ignite-3/pull/3704
> h3. Motivation
> Unfortunately, we merged PR before all the comments were published. Except
> for other formalities, there is one main note:
> {code}
> if (retryExecutor != null && matchAny(unwrapCause(errResp.throwable()),
> ACQUIRE_LOCK_ERR, REPLICA_MISS_ERR)) {
> retryExecutor.schedule(
> // Need to resubmit again to pool which is valid for synchronous
> IO execution.
> () -> partitionOperationsExecutor.execute(() ->
> res.completeExceptionally(errResp.throwable())),
> RETRY_TIMEOUT_MILLIS, MILLISECONDS);
> }
> {code}
> In the snipped, we put the response off in 20 milliseconds to retry the
> request in when the perious cercumstances are changed. But here we delay of
> handling the request intntinaly, that coud be a perfomance issue. Moreover,
> the delay does not have to apply for the replica miss exception (this
> exception is handled in client side through using the placement driver API).
> h3. Definition of done
> Excluded the handler for the replica miss exception and handled this
> exception on the client side.
> Add the logic of calculating the count of retries here, and do not delay
> response when the retry is not assumed.
> Move the timeout configuration (it cannot be a const) to the configuration
> property in the same place where the configuration of dead lock prevention is
> stored.
> Look at all the other comments in the PR
> (https://github.com/apache/ignite-3/pull/3704)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
