HTHou commented on code in PR #18569:
URL: https://github.com/apache/iotdb/pull/18569#discussion_r3912528028
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/exchange/source/SourceHandle.java:
##########
@@ -637,11 +644,19 @@ public void run() {
attempt += 1;
long startTime = System.nanoTime();
+ boolean transferAttemptRecorded = false;
try (SyncDataNodeMPPDataExchangeServiceClient client =
mppDataExchangeServiceClientManager.borrowClient(remoteEndpoint)) {
TGetDataBlockResponse resp = client.getDataBlock(req);
int tsBlockNum = resp.getTsBlocks().size();
- if (tsBlockNum == 0) {
+ if (tsBlockNum != endSequenceId - startSequenceId) {
Review Comment:
Addressed in e9aa6bd771. A non-empty response-size mismatch is recorded as
UNEXPECTED_RESPONSE_SIZE and then thrown into the existing catch path.
transferAttemptRecorded prevents a duplicate exception audit event, while the
existing retry limit and final onFailure behavior are preserved. Added
testShortResponseRetriesAndFails to verify all three attempts and terminal
failure.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/UserDataTransferProtectionMethod.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.commons.audit;
+
+public enum UserDataTransferProtectionMethod {
+ TLS,
+ UNPROTECTED;
Review Comment:
Addressed in e9aa6bd771. Renamed UNPROTECTED to NONE, updated all
references, and added a schema-name mapping test for both TLS and NONE.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/consensus/DataRegionConsensusImpl.java:
##########
@@ -143,6 +145,11 @@ private static ConsensusConfig buildConsensusConfig() {
.setThisNode(new TEndPoint(CONF.getInternalAddress(),
CONF.getDataRegionConsensusPort()))
.setTrustedChannelFailureHandler(
DNAuditLogger.getInstance()::recordTrustedChannelFailureAuditLogIfNecessary)
+ .setUserDataTransferAuditHandler(
+ COMMON_CONF.isEnableAuditLog()
+ ? DNAuditLogger.getInstance()::recordUserDataTransferAuditLog
+ : UserDataTransferAuditHandler.NO_OP)
+
.setUserDataTransferAuditClassifier(DataNodeUserDataTransferAuditor::containsUserData)
Review Comment:
Addressed in e9aa6bd771. DataRegionConsensusImpl now installs
UserDataTransferAuditClassifier.NO_USER_DATA whenever auditing is disabled,
matching the NO_OP handler gate and avoiding DataRegion lookup, request
traversal, and WAL/request deserialization.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AbstractAuditLogger.java:
##########
@@ -136,4 +138,46 @@ public void recordTrustedChannelFailureAuditLogIfNecessary(
RECORDING_TRUSTED_CHANNEL_FAILURE.remove();
}
}
+
+ /** Records one user-data transfer attempt without retaining any transferred
payload. */
+ public void recordUserDataTransferAuditLog(UserDataTransferAuditEvent event)
{
+ if (!IS_AUDIT_LOG_ENABLED
+ || event == null
+ || event.getInitiator() == null
+ || event.getSource() == null
+ || event.getTarget() == null
+ || Boolean.TRUE.equals(RECORDING_USER_DATA_TRANSFER.get())) {
+ return;
+ }
+
+ final String initiatorIdentifier =
NodeUrlUtils.convertTEndPointUrl(event.getInitiator());
+ final String sourceIdentifier =
NodeUrlUtils.convertTEndPointUrl(event.getSource());
+ final String targetIdentifier =
NodeUrlUtils.convertTEndPointUrl(event.getTarget());
+ RECORDING_USER_DATA_TRANSFER.set(true);
+ try {
+ log(
+ new AuditLogFields(
+ INTERNAL_AUDIT_LOG_USER_ID,
+ User.BUILTIN_INTERNAL_AUDIT_LOG_USERNAME,
+ initiatorIdentifier,
+ AuditEventType.USER_DATA_TRANSFER,
+ null,
Review Comment:
Addressed on the Apache side in e9aa6bd771. USER_DATA_TRANSFER audit fields
now use AuditLogOperation.CONTROL, and AbstractAuditLoggerTest verifies the
event-type/operation combination passed to the concrete logger. The
TimechoDB-only concrete event allowlist is not present in apache/iotdb, so its
CONTROL list needs the corresponding USER_DATA_TRANSFER entry in the downstream
implementation.
--
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]
