Ashwani Raina 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: (7 comments) I have some minor comments from first pass. I plan to do a second pass for more detailed review. http://gerrit.cloudera.org:8080/#/c/23246/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23246/3//COMMIT_MSG@9 PS3, Line 9: three key metrics Question - are these metrics always enabled i.e. do these metrics get populated from the get go? 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@75 PS3, Line 75: pending nit: Can this be renamed to pendingSplits for consistency? http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/main/java/org/apache/kudu/replication/wrappedsource/MetricWrappedKuduEnumerator.java@76 PS3, Line 76: unassigned ditto - unassignedSplits? http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/test/java/org/apache/kudu/replication/TestReplicationMetrics.java File java/kudu-replication/src/test/java/org/apache/kudu/replication/TestReplicationMetrics.java: http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/test/java/org/apache/kudu/replication/TestReplicationMetrics.java@51 PS3, Line 51: timeoutMs nit: Here and wherever applicable, make it "timeoutMillis" to be unambiguous http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/test/java/org/apache/kudu/replication/TestReplicationMetrics.java@98 PS3, Line 98: 15000 Any specific reason behind having timeout as 15 seconds? http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/test/java/org/apache/kudu/replication/TestReplicationMetrics.java@98 PS3, Line 98: waitForTimestampAdvancement Is there a likelihood to hit timeout for timestamp advancement on slow systems? http://gerrit.cloudera.org:8080/#/c/23246/3/java/kudu-replication/src/test/java/org/apache/kudu/replication/TestReplicationMetrics.java@125 PS3, Line 125: insertRowsIntoAllTypesTable question - I am not quite clear on "replication for table metadata", does replication taken into consideration DDL changes done via alter? -- 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: Tue, 26 Aug 2025 16:39:31 +0000 Gerrit-HasComments: Yes
