Zoltan Chovan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23246 )

Change subject: KUDU-3662 [5/n] Add metrics support to replication
......................................................................


Patch Set 3:

(1 comment)

lgtm, aside from the other reviewer's questions

http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/main/java/org/apache/kudu/replication/wrappedsource/MetricWrappedKuduEnumerator.java
File 
java/kudu-replication/src/main/java/org/apache/kudu/replication/wrappedsource/MetricWrappedKuduEnumerator.java:

http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/main/java/org/apache/kudu/replication/wrappedsource/MetricWrappedKuduEnumerator.java@129
PS3, Line 129: long[] parsed = 
HybridTimeUtil.HTTimestampToPhysicalAndLogical(hybridTime);
             :     long epochMicros = parsed[0];
             :     return epochMicros / 1_000;
> Is there a possibility for parsed to be an empty array? Or for epochMicros
HybridTimeUtil.HTTimestampToPhysicalAndLogical() doesn't throw any exceptions 
to catch. Also long values can never be null, they are primitive types not 
classes, so there is no point in checking if it's null. It might be worth to do 
some sanity checking on the actual parsed values if they are valid.



--
To view, visit http://gerrit.cloudera.org:8080/23246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfbd34c707e7539ee88863399ae3061683f8bb3b
Gerrit-Change-Number: 23246
Gerrit-PatchSet: 3
Gerrit-Owner: Marton Greber <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Thu, 28 Aug 2025 09:26:45 +0000
Gerrit-HasComments: Yes

Reply via email to