[ 
https://issues.apache.org/jira/browse/IGNITE-19536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17738072#comment-17738072
 ] 

Vyacheslav Koptilin edited comment on IGNITE-19536 at 6/28/23 11:46 AM:
------------------------------------------------------------------------

As was discussed in private, it is not so obvious the value of introducing this 
interface. At first glance, the understanding is straightforward. Moreover, 
JDBC spec has very similar exceptions:

{noformat}
8.5.2 Transient SQLExceptions 
    A Transient SQLException must extend the class SQLTransientException. 
A Transient SQLException will be thrown in situations where a previously failed 
operation might be able to succeed 
when the operation is retried without any intervention by application-level 
functionality. 
After a Transient SQLException other than SQLTransientConnectionException 
occurs, 
the application can assume that the connection is still valid. 
For SQLState class values that indicate a transient error but which are not 
specified in the following table, 
an implementation may throw an instance of the class SQLTransientException.

8.5.3 SQLRecoverableException 
    A SQLRecoverableException would be thrown in situations where the failed 
operation might succeed 
if the application performs some recovery steps and retries the entire 
transaction or 
in the case of a distributed transaction, the transaction branch. 
At a minimum, recovery includes closing the current connection and getting a 
new one. 
After a SQLRecoverableException the application must assume that the connection 
is no longer valid.
{noformat}

However, handling a particular exception depends on its type and the right way 
to write a catch block is the following:
{code:java}
    try {
        deployUnit(descriptor);
    } catch (ClassNotFoundException e) {
        ...
    } catch (DeploymentDuplicateException e) {
        ...
    } catch (InvalidDeploymentException e) {
        ...
   }
{code}

instead of 
{code:java}
    try {
        deployUnit(descriptor);
    } catch (IgniteException e) {
        if (e instanceof RecoverableException) {
             // retry deployment.
        }
    }
{code}

So, this ticket can be closed as "will not fix" until the real use cases show 
the need.


was (Author: slava.koptilin):
As was discussed in private, it is not so obvious the value of introducing this 
interface. At first glance, the understanding is straightforward. Moreover, 
JDBC spec has very similar exceptions:

{noformat}
8.5.2 Transient SQLExceptions 
    A Transient SQLException must extend the class SQLTransientException. 
A Transient SQLException will be thrown in situations where a previously failed 
operation might be able to succeed 
when the operation is retried without any intervention by application-level 
functionality. 
After a Transient SQLException other than SQLTransientConnectionException 
occurs, 
the application can assume that the connection is still valid. 
For SQLState class values that indicate a transient error but which are not 
specified in the following table, 
an implementation may throw an instance of the class SQLTransientException.

8.5.3 SQLRecoverableException 
    A SQLRecoverableException would be thrown in situations where the failed 
operation might succeed if the application performs some recovery steps and 
retries the entire transaction or in the case of a distributed transaction, the 
transaction branch. At a minimum, recovery includes closing the current 
connection and getting a new one. After a SQLRecoverableException the 
application must assume that the connection is no longer valid.
{noformat}

However, handling a particular exception depends on its type and the right way 
to write a catch block is the following:
{code:java}
    try {
        deployUnit(descriptor);
    } catch (ClassNotFoundException e) {
        ...
    } catch (DeploymentDuplicateException e) {
        ...
    } catch (InvalidDeploymentException e) {
        ...
   }
{code}

instead of 
{code:java}
    try {
        deployUnit(descriptor);
    } catch (IgniteException e) {
        if (e instanceof RecoverableException) {
             // retry deployment.
        }
    }
{code}

So, this ticket can be closed as "will not fix" until the real use cases show 
the need.

> Introduce a "recoverable" flag to differentiate recoverable and 
> non-recoverable exceptions
> ------------------------------------------------------------------------------------------
>
>                 Key: IGNITE-19536
>                 URL: https://issues.apache.org/jira/browse/IGNITE-19536
>             Project: Ignite
>          Issue Type: Improvement
>            Reporter: Vyacheslav Koptilin
>            Assignee: Vyacheslav Koptilin
>            Priority: Major
>              Labels: iep-84, ignite-3
>             Fix For: 3.0.0-beta2
>
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> It seems useful to introduce a marker interface in order to differentiate 
> recoverable and non-recoverable errors. This approach should simplify 
> exception handling on the client side.
> Something as follows:
> {code:java}
>         try {
>             igniteCompute.execute();
>         }
>         catch (IgniteComputeException error) {
>             if (error instanceof RecoverableException) {
>                 // Put retry logic here.
>             }
>         }
> {code}
> I think that introducing a marker interface is better than introducing a new 
> method in IgniteException/IgniteCheckedException (boolean isRecoverable()) 
> from the user's point of view: it is easier to document and understand which 
> error is recoverable or not.



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

Reply via email to