apurtell commented on PR #2556: URL: https://github.com/apache/phoenix/pull/2556#issuecomment-4835179237
The precommit build https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-2556/ had four failures. All are not related to this change. /cc @virajjasani 1. org.apache.phoenix.end2end.CDCQueryIT.testSelectWithTimeRange > java.lang.IllegalArgumentException: bound must be positive > at java.util.Random.nextInt(Random.java:388) > at org.apache.phoenix.end2end.CDCQueryIT.testSelectWithTimeRange(CDCQueryIT.java:786) `CDCQueryIT` is flaky because it does not always provide a positive bound to `Random.nextInt()`. This is unrelated to the EXPLAIN changes. 2. org.apache.phoenix.end2end.MetadataServerConnectionsIT.testViewCreationAndServerConnections > org.apache.hadoop.hbase.DoNotRetryIOException: Not allowed > at org.apache.phoenix.end2end.MetadataServerConnectionsIT$TestMetaDataEndpointImpl.createTable(MetadataServerConnectionsIT.java:183) > at org.apache.phoenix.coprocessor.generated.MetaDataProtos$MetaDataService.callMethod(MetaDataProtos.java:17539) > at org.apache.hadoop.hbase.regionserver.HRegion.execService(HRegion.java:7992) The code in `MetadataServerConnectionsIT` calls `Thread.activeCount()` to get a thread count, allocates an array of that size, then calls `Thread.enumerate()` to fill it. However, `Thread.enumerate()` can return fewer threads than the array size if threads exit between the two calls, leaving null slots in the array. When the stream tries to call `getName()` on these null entries, it throws an NPE. This is unrelated to the EXPLAIN changes. 3. org.apache.phoenix.end2end.UpsertSelectIT.testUpsertSelectOnDescToAsc > java.lang.AssertionError: expected:<0> but was:<1> > at org.junit.Assert.fail(Assert.java:89) > at org.junit.Assert.failNotEquals(Assert.java:835) > at org.junit.Assert.assertEquals(Assert.java:647) > at org.junit.Assert.assertEquals(Assert.java:633) > at org.apache.phoenix.end2end.UpsertSelectIT.assertNoConnLeak(UpsertSelectIT.java:113) The connection leak is a preexisting intermittent connection cleanup race in the client-side UPSERT SELECT parallel mutating iterator path. This PR does not modify any of the files that govern this connection lifecycle. The PR's `UpsertCompiler` edits only add a `returningRow` field, restructure the `getExplainPlan()` text, and pass a pre-built `StatementContext.forRewrite(...)` through `SubselectRewriter.flatten` / `SubqueryRewriter.transform`. None of those code paths runs during `executeUpdate()`. They only run when the user (or test) asks for an explicit EXPLAIN. This is unrelated to the EXPLAIN changes. 4. org.apache.phoenix.monitoring.PhoenixTableLevelMetricsIT.testHistogramMetricsForQueries > java.lang.AssertionError > at org.junit.Assert.fail(Assert.java:87) > at org.junit.Assert.assertTrue(Assert.java:42) > at org.junit.Assert.assertTrue(Assert.java:53) > at org.apache.phoenix.monitoring.PhoenixTableLevelMetricsIT.assertHistogramMetricsForQueries(PhoenixTableLevelMetricsIT.java:1595) The issue is that the `SelfHealingTask` running on a background thread is making its own queries to system tables between when the test resets the global metrics and when it runs its own query, so the global query time ends up capturing both the background task's work and the test's work, causing the assertion to fail because the recorded max latency value is larger than expected. This is unrelated to the EXPLAIN changes. --- I think this change is good to merge. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
