ascherbakoff commented on code in PR #5752:
URL: https://github.com/apache/ignite-3/pull/5752#discussion_r2084084041
##########
modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java:
##########
@@ -511,10 +518,13 @@ private void handlePartitionAssignmentChange(int flags,
ClientMessageUnpacker un
private void handleNotification(long id, ClientMessageUnpacker unpacker,
@Nullable Throwable err) {
// One-shot notification handler - remove immediately.
CompletableFuture<PayloadInputChannel> handler =
notificationHandlers.remove(id);
+
if (handler == null) {
- log.error("Unexpected notification ID [remoteAddress=" +
cfg.getAddress() + "]: " + id);
+ // Default notification handler. Used to deliver delayed
replication acks.
Review Comment:
No, we can't. A notification might be not delivered depending on server side
condition (no-op operation doesn't produce notifications).
Even if we could, I would keep it this way to avoid additional overhead on
creating and removing CHM entry without any need.
And this doesn't look dirty to me. Server-side delayed acks implemented in
the same way, see org.apache.ignite.internal.tx.impl.TxManagerImpl#onReceived
The procotol is easily extendable in the future using feature flags.
--
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
