Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/14601 )
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
......................................................................
Patch Set 6:
(3 comments)
> Looks like a Java test broke.
Yep, indeed. And I suspect it happens 100% times, so it's not a flake:
There was 1 failure:
1) testGetTableStatistics(org.apache.kudu.client.TestKuduTable)
java.lang.AssertionError
at org.junit.Assert.fail(Assert.java:86)
at org.junit.Assert.assertTrue(Assert.java:41)
at org.junit.Assert.assertTrue(Assert.java:52)
at
org.apache.kudu.client.TestKuduTable.testGetTableStatistics(TestKuduTable.java:818)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at
org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
at
org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.lang.Thread.run(Thread.java:748)
FAILURES!!!
Tests run: 16, Failures: 1
http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/catalog_manager.cc@5591
PS6, Line 5591: will trigger
nit: makes
http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/catalog_manager.cc@5599
PS6, Line 5599: !old_stats.has_on_disk_size()
It's not easy to follow the logic of these conditionals without knowing the
history of the 'disk_size' and 'live_row_count' metrics. Could you add a small
comment explaining why the absence of the 'on_disk_size' metric in old_stats
means the new reported stat doesn't support 'live_row_count'?
http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/master-test.cc@1824
PS6, Line 1824: //old_stats.set_on_disk_size(1);
Remove this commented out line, it's not needed.
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 6
Gerrit-Owner: helifu <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: helifu <[email protected]>
Gerrit-Comment-Date: Thu, 28 Nov 2019 04:24:59 +0000
Gerrit-HasComments: Yes