Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21045 )

Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong 
number
......................................................................


Patch Set 1:

(3 comments)

> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10303/

Thanks for contributing to Impala! There is a test failure 
(testEventProcessorMetricsForSkippedMetric) that need to investigate:
https://jenkins.impala.io/job/ubuntu-20.04-from-scratch/1805/testReport/junit/org.apache.impala.catalog.events/MetastoreEventsProcessorTest/testEventProcessorMetricsForSkippedMetric/

java.lang.AssertionError: two more events should have been received
        at org.junit.Assert.fail(Assert.java:88)
        at org.junit.Assert.assertTrue(Assert.java:41)
        at 
org.apache.impala.catalog.events.MetastoreEventsProcessorTest.testEventProcessorMetricsForSkippedMetric(MetastoreEventsProcessorTest.java:2046)
        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.RunBefores.evaluate(RunBefores.java:26)
        at 
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at 
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
        at 
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at 
org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
        at 
org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
        at 
org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
        at 
org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
        at 
org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
        at 
org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
        at 
org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
        at 
org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1055
PS1, Line 1055:         } else if (numPartsRefreshed == -1) {
I think using "else" without the condition is more robust in case the returned 
value changes to another negative value in the future.


http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1056
PS1, Line 1056: inc
Please add a debug log when increasing this counter.


http://gerrit.cloudera.org:8080/#/c/21045/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2341
PS1, Line 2341:             debugLog("Incremented skipped metric to " + metrics_
We can improve this to add the reason - "since no partitions were added"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a
Gerrit-Change-Number: 21045
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <cclive1...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 08:13:47 +0000
Gerrit-HasComments: Yes

Reply via email to