[jira] [Updated] (IGNITE-17602) Trace identifier of an exception can be lost during transferring ReplicaRespone from replica node to ReplicaService

["Vyacheslav Koptilin (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IGNITE-17602?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vyacheslav Koptilin updated IGNITE-17602:
-----------------------------------------
    Description: 
Current implementation of processing ReplicaResponse has the following drawbacks:

 - if sending ReplicaRequest failed for some reason, ReplicaService just throw new exception without preserving traceId if it exists
{code:java}
return messagingService.invoke(node.address(), req, RPC_TIMEOUT).handle((response, throwable) -> {
    if (throwable != null) {
        if (throwable instanceof TimeoutException) {
            throw new ReplicationTimeoutException(req.groupId());
        } else if (throwable instanceof PrimaryReplicaMissException) {
            throw (PrimaryReplicaMissException) throwable;
        }

        throw new ReplicationException(req.groupId(), throwable); <- original traceId will be lost
{code}

 - ErrorReplicaResponse explicitly transfer error code of exception, its traceId etc. It seems to me, we can just transfer the exception (our messaging service properly marshal/unmarshal throwables)

 - org.apache.ignite.internal.replicator.exception.ExceptionUtils does not care about IgniteInternalChackedException. For instance LockException (which is checked exception) is transformed to IgniteInternalException.

In addition, TransactionImpl does not properly handle ExecutionException:
{code:java}
    @Override
    public void commit() throws TransactionException {
        try {
            commitAsync().get();
        } catch (ExecutionException e) {
            if (e.getCause() instanceof TransactionException) {
                throw (TransactionException) e.getCause();
            } else {
                throw new TransactionException(e.getCause());
            }
        } catch (Exception e) {
            throw new TransactionException(e);
        }
    }
{code}
We should not re-throw e.getCause() because we will lost a stack frame related to ExecutionException, and therefore the resulting stack trace will not show a code path/method that called commit().

> Trace identifier of an exception can be lost during transferring ReplicaRespone from replica node to ReplicaService
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: IGNITE-17602
>                 URL: https://issues.apache.org/jira/browse/IGNITE-17602
>             Project: Ignite
>          Issue Type: Bug
>            Reporter: Vyacheslav Koptilin
>            Assignee: Vyacheslav Koptilin
>            Priority: Major
>              Labels: ignite-3
>
> Current implementation of processing ReplicaResponse has the following drawbacks:
>  - if sending ReplicaRequest failed for some reason, ReplicaService just throw new exception without preserving traceId if it exists
> {code:java}
> return messagingService.invoke(node.address(), req, RPC_TIMEOUT).handle((response, throwable) -> {
>     if (throwable != null) {
>         if (throwable instanceof TimeoutException) {
>             throw new ReplicationTimeoutException(req.groupId());
>         } else if (throwable instanceof PrimaryReplicaMissException) {
>             throw (PrimaryReplicaMissException) throwable;
>         }
>         throw new ReplicationException(req.groupId(), throwable); <- original traceId will be lost
> {code}
>  - ErrorReplicaResponse explicitly transfer error code of exception, its traceId etc. It seems to me, we can just transfer the exception (our messaging service properly marshal/unmarshal throwables)
>  - org.apache.ignite.internal.replicator.exception.ExceptionUtils does not care about IgniteInternalChackedException. For instance LockException (which is checked exception) is transformed to IgniteInternalException.
> In addition, TransactionImpl does not properly handle ExecutionException:
> {code:java}
>     @Override
>     public void commit() throws TransactionException {
>         try {
>             commitAsync().get();
>         } catch (ExecutionException e) {
>             if (e.getCause() instanceof TransactionException) {
>                 throw (TransactionException) e.getCause();
>             } else {
>                 throw new TransactionException(e.getCause());
>             }
>         } catch (Exception e) {
>             throw new TransactionException(e);
>         }
>     }
> {code}
> We should not re-throw e.getCause() because we will lost a stack frame related to ExecutionException, and therefore the resulting stack trace will not show a code path/method that called commit().



--
This message was sent by Atlassian Jira
(v8.20.10#820010)