OperationGroup extends AutoCloseable

[Douglas Surber]

Yes, all the calls are non-blocking. The close doesn’t “terminate the group”. It notifies the implementation that no more member Operations will be added. It doesn’t stop anything from executing.

What I didn’t discuss but should have is the case where members are added as a result of the execution of other Operations.

  OperationGroup grp = session.operationGroup();
  grp.submit();
  grp.rowOperation(selectSql)
    .collect(Collector.of( () -> null,
                                    (n, c) -> grp.rowCountOperation(insertSql)
                                                     .set(“param”, c.at <http://c.at/>(“index”).get(Integer.class), AdbaType.INTEGER)
                                                     .submit(),
                                   (l, r) -> l,
                                   n -> { grp.close(); return n;}))
     .submit();

The above does an insert for each row selected. The OperationGroup cannot be closed by try with resources because it would be closed before all (any) of the inserts are added. The problem with this pattern is failing to call grp.close() in the finalize method. If that happens the OperationGroup will never be completed and execution will hang. 

This was the code pattern that motivated the ability to add members after submit, but I don’t believe it will be the most common use case. It’s unfortunate that it doesn’t fail overtly rather than hanging, but the current design is no better in this regard. It just has more explicit method names. I don’t think those method names carry their weight compared to using try with resources.

You can write exactly the code above with a RowPublisherOperation. It takes a few more lines of code to define the Subscriber, but it’s not hard.

Douglas

> On Jul 19, 2018, at 11:11 AM, Dávid Karnok <akarnokd at gmail.com> wrote:
> 
> Aren't those submit calls (supposed) asynchronous? In that case, won't the try-with-resources terminate the group before, most likely, anything could even happen?
> 
> The scenario sounds like typical multi-valued asynchronous flow, for which a Flow.Publisher<Submission> source would be able to generate more submissions as well as indicate there won't be any further submissions.
> 
> Douglas Surber <douglas.surber at oracle.com <mailto:douglas.surber at oracle.com>> ezt írta (időpont: 2018. júl. 19., Cs, 19:46):
> 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
> 
> 
> 
> 
> -- 
> Best regards,
> David Karnok