undefined | Better HN

0 pointsAdieuToLogic6mo ago0 comments

> Having the retry logic within the `finally` block seems like the best way of denoting the intention to me, since the intention is to swallow the result, whether that was a return or a throw.

Except that is not the documented intent of the `finally` construct:

  The finally block always executes when the try block exits. 
  This ensures that the finally block is executed even if an 
  unexpected exception occurs. But finally is useful for more 
  than just exception handling — it allows the programmer to 
  avoid having cleanup code accidentally bypassed by a 
  return, continue, or break. Putting cleanup code in a 
  finally block is always a good practice, even when no 
  exceptions are anticipated.[0]

Using `finally` for implementing retry logic can be done, as you have illustrated, but that does not mean it is "the best way of denoting the intention." One could argue this is a construct specific to Java (the language) and does not make sense outside of this particular language-specific idiom.

Conceptually, "retries" are not "cleanup code."

0 - https://docs.oracle.com/javase/tutorial/essential/exceptions...

0 comments

maxdamantus6mo ago

Sounds like the right intent to me. To pinpoint your existing quote from the documentation:

> The finally block always executes when the try block exits. This ensures that the finally block is executed even if an unexpected exception occurs.

The intent of the transaction code is that the consistency is checked (using `tx.commit()`) "even if an unexpected exception occurs".

I'm not sure how else to interpret that to be honest. If you've got a clearer way of expressing this, feel free to explain.

locknitpicker6mo ago

> The intent of the transaction code is that the consistency is checked (using `tx.commit()`) "even if an unexpected exception occurs".

A transaction failing is the opposite of an unexpected event. Transactions failing is a central use case of any transaction. Therefore it should be handled explicitly instead of using exceptions.

Exceptions are for unexpected events such as the node running out of memory, or a process failing to write to disk.

maxdamantus6mo ago

> A transaction failing is the opposite of an unexpected event.

That's why it's denoted by a non-exceptional return value from `tx.commit()` in my sample code. When I've talked about exceptions here, I'm talking about exceptions raised within the transaction. If the transaction succeeds, those exceptions should be propagated to the calling code.

> Exceptions are for unexpected events such as the node running out of memory, or a process failing to write to disk.

Discussing valid uses of exceptions seems orthogonal to this (should OOM lead to a catchable exception [0], or should it crash the process?). In any case, if the process is still alive and the transaction code determines without error that "yes, this transaction was invalid due to other contending transactions", it should retry the transaction. If something threw due to lack of memory or disk space, chances are it will throw again within a successful transaction and the error will be propagated.

[0] As alluded to in my first post, you might want to add some special cases for exceptions/errors that you want to immediately propagate instead of retrying. Eg, you might treat `Error` subtypes differently, which includes `OutOfMemoryError` and other cases that suggest the program is in a potentially unusable state, but this still isn't required according to the intent of the transactional logic.

AdieuToLogicOP6mo ago

> The intent of the transaction code is that the consistency is checked (using `tx.commit()`) "even if an unexpected exception occurs".

First, having a `commit` unconditionally attempted when an exception is raised would surprise many developers. Exceptions in transactional logic are often used to represent a "rollback persistent store changes made thus far" scenario.

Second, using a condition within `finally` to indicate a retry due to a `commit` failing could be expressed in a clearer manner by having it within the `try` block as described by IntelliJ here[0].

0 - https://www.jetbrains.com/help/inspectopedia/ContinueOrBreak...

maxdamantus6mo ago

> Exceptions in transactional logic are often used to represent a "rollback persistent store changes made thus far" scenario.

Handling can be added to change the transaction to be read-only if the inner code throws a particular exception, but the consistency should still be checked through a `commit` phase (at least in an OCC setting), so the `continue` in `finally` is still the correct way to do it.

> could be expressed in a clearer manner by having it within the `try` block as described by IntelliJ here[0].

> 0 - https://www.jetbrains.com/help/inspectopedia/ContinueOrBreak...

Wrong link? The only solution I see there is to add a comment to suppress the warning, which sounds fine to me (eg, analogous to having a `// fallthrough` comment when intentionally omitting `break` statements within `switch`, since I can agree that both of these things are uncommon, but sometimes desirable).

AdieuToLogicOP6mo ago

> Handling can be added to change the transaction to be read-only if the inner code throws a particular exception, but the consistency should still be checked through a `commit` phase (at least in an OCC setting), so the `continue` in `finally` is still the correct way to do it.

This approach fails to account for `fn` performing multiple mutations where an exception is raised from statement N, where N > 1.

For example, suppose `fn` successfully updates a record in table `A`, then attempts to insert a record into table `B` which produces a constraint violation exception[0]. Unconditionally performing a `commit` in the `finally` block will result in the mutation in table `A` being persisted, thus resulting in an invalid system persistent state.

If the `try` block performed the `commit` and the `finally` block unconditionally performed a `rollback`, then the behavior I believe sought would be sound.

>> 0 - https://www.jetbrains.com/help/inspectopedia/ContinueOrBreak...

> Wrong link?

No, it's the link I intended. The purpose of it was to provide the warning anyone working in a translation unit using the technique originally proffered would see as well as be a starting point for research.

0 - https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/...

j / k navigate · click thread line to collapse

0 comments

maxdamantus6mo ago

Sounds like the right intent to me. To pinpoint your existing quote from the documentation:

> The finally block always executes when the try block exits. This ensures that the finally block is executed even if an unexpected exception occurs.

The intent of the transaction code is that the consistency is checked (using `tx.commit()`) "even if an unexpected exception occurs".

I'm not sure how else to interpret that to be honest. If you've got a clearer way of expressing this, feel free to explain.

locknitpicker6mo ago

> The intent of the transaction code is that the consistency is checked (using `tx.commit()`) "even if an unexpected exception occurs".

A transaction failing is the opposite of an unexpected event. Transactions failing is a central use case of any transaction. Therefore it should be handled explicitly instead of using exceptions.

Exceptions are for unexpected events such as the node running out of memory, or a process failing to write to disk.

maxdamantus6mo ago

> A transaction failing is the opposite of an unexpected event.

> Exceptions are for unexpected events such as the node running out of memory, or a process failing to write to disk.

AdieuToLogicOP6mo ago

> The intent of the transaction code is that the consistency is checked (using `tx.commit()`) "even if an unexpected exception occurs".

Second, using a condition within `finally` to indicate a retry due to a `commit` failing could be expressed in a clearer manner by having it within the `try` block as described by IntelliJ here[0].

0 - https://www.jetbrains.com/help/inspectopedia/ContinueOrBreak...

maxdamantus6mo ago

> Exceptions in transactional logic are often used to represent a "rollback persistent store changes made thus far" scenario.

> could be expressed in a clearer manner by having it within the `try` block as described by IntelliJ here[0].

> 0 - https://www.jetbrains.com/help/inspectopedia/ContinueOrBreak...

AdieuToLogicOP6mo ago

This approach fails to account for `fn` performing multiple mutations where an exception is raised from statement N, where N > 1.

If the `try` block performed the `commit` and the `finally` block unconditionally performed a `rollback`, then the behavior I believe sought would be sound.

>> 0 - https://www.jetbrains.com/help/inspectopedia/ContinueOrBreak...

> Wrong link?

0 - https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/...

j / k navigate · click thread line to collapse