vldpyatkov commented on code in PR #3704:
URL: https://github.com/apache/ignite-3/pull/3704#discussion_r1598649440
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaService.java:
##########
@@ -210,7 +225,14 @@ private <R> CompletableFuture<R> sendToReplica(String
targetNodeConsistentId, Re
return null;
});
} else {
- res.completeExceptionally(errResp.throwable());
+ if (retryExecutor != null &&
matchAny(unwrapCause(errResp.throwable()), ACQUIRE_LOCK_ERR, REPLICA_MISS_ERR))
{
+ retryExecutor.schedule(
+ // Need to resubmit again to pool which is
valid for synchronous IO execution.
+ () ->
partitionOperationsExecutor.execute(() ->
res.completeExceptionally(errResp.throwable())),
+ RETRY_TIMEOUT_MILLIS, MILLISECONDS);
Review Comment:
If I understood it correctly,, we are holding response for several
milliseconds to prevent instantaneous retry.
What will change if we don't do that? Because it is incorrect to delay a
response instead of retrying, especially in cases where we don't make the retry
again.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -626,50 +621,71 @@ private <R> CompletableFuture<R> trackingInvoke(
|| request instanceof MultipleRowPkReplicaRequest &&
((MultipleRowPkReplicaRequest) request).requestType() != RW_GET_ALL
|| request instanceof SwapRowReplicaRequest;
- if (write && !full) {
- // Track only write requests from explicit transactions.
- if (!transactionInflights.addInflight(tx.id(), false)) {
- return failedFuture(
- new TransactionException(TX_ALREADY_FINISHED_ERR,
format(
- "Transaction is already finished
[tableName={}, partId={}, txState={}].",
- tableName,
- partId,
- tx.state()
- )));
- }
+ if (full) { // Full transaction retries are handled in postEnlist.
+ return replicaSvc.invoke(primaryReplicaAndConsistencyToken.get1(),
request);
+ } else {
+ if (write) { // Track only write requests from explicit
transactions.
Review Comment:
`} else if (write) {`
This code format is acceptable here.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -1022,6 +1028,10 @@ private static boolean
allSchemaVersionsSame(Collection<? extends BinaryRow> row
boolean first = true;
for (BinaryRow row : rows) {
+ if (row == null) {
Review Comment:
It looks like a bug, how is it happening?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
