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

Reply via email to