rpuch commented on code in PR #5241:
URL: https://github.com/apache/ignite-3/pull/5241#discussion_r1963302478
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java:
##########
@@ -197,15 +193,15 @@ CompletableFuture<Void> cleanup(
* <p>The nodes to sends the request to are calculated by the placement
driver.
*
* @param commitPartitionId Commit partition id.
- * @param enlistedPartitions Enlisted partition groups.
+ * @param enlistedPartitions Enlisted partitions.
* @param commit {@code true} if a commit requested.
* @param commitTimestamp Commit timestamp ({@code null} if it's an abort).
* @param txId Transaction id.
* @return Completable future of Void.
*/
CompletableFuture<Void> cleanup(
TablePartitionId commitPartitionId,
- Collection<ReplicationGroupId> enlistedPartitions,
+ Collection<EnlistedPartitionGroup> enlistedPartitions,
Review Comment:
These 3 represent similar information ('partition enlistment information')
at different times in different contexts. They all relate to some groupId (but
in 2 cases groupId is stored externally, as map key, and in the last case it's
stored inside) and they all have `tableIds` (in first case it's mutable, in
other cases it's not).
1. `MutablePartitionEnlistment` (which I've just renamed to
`OngoingTxPartitionEnlistment` to make it more clear what it is) is used to
track enlistment information for a partition (which groupId is stored
externally) while the transaction is still running, before it is finished; so
it needs to know its primary replica (in the form of `ClusterNode`, not just
name) and enlistment token
2. `FinishingPartitionEnlistment` represents the same information when the
transaction is being finished and when tx cleanup happens. At this stage,
`tableIds` doesn't need to be mutable anymore, and token is not needed; the
primary node is still needed but it can be represented as a name.
3. `EnlistedPartitionGroup` represents this 'thing' stored in `TxMeta` (and
in tx state storage). Primary information is not needed anymore, but groupId
has to be stored explicitly. Even if we represented collections of
`EnlistedPartitionGroup` as maps from groupIds to the remaining enlistment
information, we wouldn't be able to reuse `FinishingPartitionEnlistment`
because we don't primary node name it contains. (Technically, we could, but it
would be ugly). On the other hand, I don't look the idea to remove
`EnlistedPartitionGroup` and just use maps from groupIds to tableIds everywhere
we use collections of `EnlistedPartitionGroup` (this would be hide an entity)
To sum up: I wasn't able to invent any way to remove any of these entities.
I named them to make first and second look alike (as they are really close) and
third to be a little different (as it's context is a bit different). If you
have any ideas on how to improve this, please share them.
I also updated javadocs on the classes to decrease possible confusion.
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
