gaborgsomogyi commented on a change in pull request #26935:
URL: https://github.com/apache/spark/pull/26935#discussion_r512543408
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala
##########
@@ -89,16 +116,16 @@ trait StateStore {
def commit(): Long
/**
- * Abort all the updates that have been made to the store. Implementations
should ensure that
- * no more updates (puts, removes) can be after an abort in order to avoid
incorrect usage.
+ * Return an iterator containing all the key-value pairs in the StateStore.
Implementations must
+ * ensure that updates (puts, removes) can be made while iterating over this
iterator.
*/
- def abort(): Unit
+ override def iterator(): Iterator[UnsafeRowPair]
/**
- * Return an iterator containing all the key-value pairs in the StateStore.
Implementations must
- * ensure that updates (puts, removes) can be made while iterating over this
iterator.
+ * Abort all the updates that have been made to the store. Implementations
should ensure that
+ * no more updates (puts, removes) can be after an abort in order to avoid
incorrect usage.
*/
- def iterator(): Iterator[UnsafeRowPair]
+ override def abort(): Unit
Review comment:
Thanks for the example! After had a second look + read your explanation
I think it's fine.
My rationale: Since the following contract applies both cases the given
example is fine: `either commit or abort called, which must free up resources
with double free guard`. In the mentioned example an internal `protected def
close...` can be added to A which can be called from `B.commit` and `B.abort`.
The mentioned `close` can contain double guard so with the actual API proper
solution can be implemented. I agree that adding close would be more clear but
considering the complexity I agree with you guys.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
