Re: [PR] Implement UDP transport for air-gap sink [iotdb]

["hongzhi-gao (via GitHub)" <gi...@apache.org>]

hongzhi-gao commented on PR #17954:
URL: https://github.com/apache/iotdb/pull/17954#issuecomment-6208257268

   <!-- iotdb-ai-radar-review:review-apache-iotdb-17954-4903676cbd2b283748af499847c605bfe6e4b066 -->
   
   **Automated preliminary correctness review** (Codex CLI, based on PR head `4903676cbd2b`). **No tests were run**.
   
   1. [iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/sink/protocol/IoTDBAirGapSink.java:508](https://github.com/apache/iotdb/blob/4903676cbd2b283748af499847c605bfe6e4b066/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/sink/protocol/IoTDBAirGapSink.java#L508)
      - **Trigger:** UDP duplicates an OK acknowledgement for request A. After consuming the first copy, the sender sends request B, which is lost, and receives the duplicate acknowledgement.
      - **Potential issue:** The response contains only a status byte, and sendOneDatagram accepts it without correlating it with the current request. Consequently, B returns success although the receiver never received it. Serial sending and a connected DatagramSocket do not prevent duplicated or delayed datagrams from the same peer. Requests and acknowledgements need matching identifiers to prevent false success and data loss.
   
   2. [iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/receiver/protocol/airgap/IoTDBAirGapReceiverAgent.java:118](https://github.com/apache/iotdb/blob/4903676cbd2b283748af499847c605bfe6e4b066/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/receiver/protocol/airgap/IoTDBAirGapReceiverAgent.java#L118)
      - **Trigger:** UDP senders reconnect using new source ports, or datagrams arrive from many distinct endpoints while the service remains running.
      - **Potential issue:** Each endpoint permanently adds a ClientSession to udpClientSessions before packet validation. Entries are removed only by stopping the entire service; removing the current thread's session after a packet does not remove this map entry. Thus even malformed packets accumulate retained sessions without a bound or expiry. Receiver state created for valid clients likewise has no endpoint retirement path here.
   
   This review is AI-assisted and not an approval or request for changes. Please verify each scenario before acting.


-- 
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: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org