OperationGroup extends AutoCloseable

[Douglas Surber]

Connection, now renamed Session, extends AutoClosable already. Making OperationGroup extend AutoCloseable should have no impact on Session.

While the JavaDoc for AutoCloseable.close talks about relinquishing underlying resources, that is not precisely what OperationGroup.close or Session.close do. As ADBA is an async API many things don’t happen when a method is called. Instead the method queues an action to happen in the future. And at some level of abstraction thats what both close methods do. At the very least the usage pattern of OperationGroup.close and Session.close matches AutoCloseable.close and try with resources. Even if OperationGroup.close and Session.close don’t release resources at some level of abstraction they are sufficiently similar to AutoCloseable.close to insure that having the same name will not cause confusion.

Basically try with resources does exactly the right thing for OperationGroup and Session.

Douglas

> On Jul 20, 2018, at 8:50 AM, Mark Rotteveel <mark at lawinegevaar.nl> wrote:
> 
> On thing I am wondering about is if making OperationGroup extend AutoCloseable is compatible with Connection, as that is an OperationGroup, and has a more specific contract for close().
> 
> Mark
> 
> On 2018-07-19 19:45, Douglas Surber wrote:
>> OperationGroup has a problem. It needs to be possible to add more
>> member Operations to an OperationGroup after it is submitted. It is
>> also necessary to know when all member Operations have been added as
>> the OperationGroup cannot be completed until it knows no more member
>> Operations will be added. Finally we want to minimize the likelyhood
>> of Session execution hanging because an OperationGroup is waiting for
>> more members when there are none.
>> I approached this problem by specifying that by default no more member
>> Operations can be added after the OperationGroup is submitted. For the
>> case where members need to be added after submission there are two
>> special methods.
>> public Submission<T> submitHoldingForMoreMembers();
>> public Submission<T> releaseProhibitingMoreMembers();
>> The long, descriptive names are intentional. The goal is to clearly
>> remind users that they must release the OperationGroup if they hold
>> it. I’ve never liked this approach.
>> I’d like to offer an alternative.
>> - Remove submitHoldingForMoreMembers and releaseProhibitingMoreMembers.
>> - Define OperationGroup to implement AutoCloseable.
>> - Member Operations can be added to an open OperationGroup.
>> - An OperationGroup will not be completed until after it is closed.
>> - An OperationGroup must be submitted before it is closed.
>> try (OperationGroup grp = session.operationGroup()) {
>>  grp.rowCountOperation(insertSql)
>>     .submit();
>>  CompletionStage result = grp.submit().getCompletionStage();
>>  grp.rowCountOperation(updateSql)
>>     .submit();
>> }
>> A single usage pattern using try with resources seems much more
>> reliable that the multiple usage patters with unfamiliar methods. Yes,
>> the default case
>>  OperationGroup grp = session.operationGroup();
>>  grp.rowCountOperation(insertSql)
>>     .submit();
>>  grp.rowCountOperation(updateSql)
>>     .submit();
>>  return grp.submit().getCompletionStage();
>> is perhaps simpler, but not by much.
>> try (OperationGroup grp = session.operationGroup()) {
>>  grp.rowCountOperation(insertSql)
>>     .submit();
>>  grp.rowCountOperation(updateSql)
>>     .submit();
>>  return grp.submit().getCompletionStage();
>> }
>> I’m starting to believe that the default case is not going to be the
>> most common case. I think held OperationGroup are going to be much
>> more common than I initially believed. A “held” group would look very
>> similar to the “default” case.
>> try (OperationGroup grp = session.operationGroup()) {
>>  CompletionStage result = grp.submit().getCompletionStage();
>>  grp.rowCountOperation(insertSql)
>>     .submit();
>>  grp.rowCountOperation(updateSql)
>>     .submit();
>>  return result;
>> }
>> The only difference is when the OperationGroup is submitted. If
>> OperationGroups are always written using try with resources the
>> forgetting to close one will be rare and easily noticed in code
>> reviews. The tiny advantage of the default case, not having to call
>> close, doesn’t seem to buy much compared to the complexity the current
>> model adds to the held case. Uniform use of try with resources seems a
>> much better approach.
>> Thoughts?
>> Douglas