Don't use 2PC if COPY operates on a single shard - #7187
Don't use 2PC if COPY operates on a single shard#7187Naisila Puka (naisila) wants to merge 1 commit into
Conversation
See discussion in: #7160 (comment)
e7d77ae to
6e19d7b
Compare
| /* all modifications use 2PC */ | ||
| Use2PCForCoordinatedTransaction(); | ||
| /* all modifications use 2PC unless we only touch a single shard */ | ||
| if (!IsCitusTableTypeCacheEntry(cacheEntry, SINGLE_SHARD_DISTRIBUTED) && |
There was a problem hiding this comment.
Can't we check if shardIntervalList has more than 1 shards?
There was a problem hiding this comment.
Yeah, I also think that we should consider generalizing the code.
I think we should probably postpone the 2PC decision until CitusCopyDestReceiverShutdown(), there iterate over shardStateHash and make sure that only a single node is involved (CopyShardState->placementStateList->groupId).
There was a problem hiding this comment.
probably
I said probably because I cannot exactly remember whether we ever clear/invalidate CopyShardState before copy finishes. If so, we cannot follow this suggestion
There was a problem hiding this comment.
we probably need similar checks as in Activate2PCIfModifyingTransactionExpandsToNewNode to handle cases where the transaction already did stuff on other nodes
(the current change seems potentially faulty)
4 participants
See discussion in:
#7160 (comment)
Fixes #7176