FrankChen021 commented on PR #20291: URL: https://github.com/apache/druid/pull/20291#issuecomment-5742284870
Re the remaining P2 (`BrokerSegmentMetadataCache.java:268`, empty-signature removal metric): good catch, addressed in a59f5f5d4d. `buildDataSourceRowSignature` returns an empty but non-null signature while a datasource only holds unrefreshed segments, and that branch still emitted `dataSource/removed` unconditionally. It now uses the same `tables.remove(dataSource) != null` guard as the null-signature branch and the last-segment callback, so whichever path actually removes the table is the only one that emits. As a side effect, a new datasource whose schema has not been initialised no longer reports a removal on every refresh cycle when no table ever existed. Tests added: - `testLastSegmentRemovedDuringRefreshWithUninitializedSegmentEmitsRemovalOnce` drives the exact interleaving: the table is built from a refreshed segment, an unrefreshed segment is added and the refreshed one removed, a refresh is started on another thread and held (via an overridden `buildDataSourceRowSignature`) right after computing the empty signature, the last segment is removed on the test thread, then the refresh is released. Asserts exactly one `dataSource/removed` and no `foo` table. - `testRefreshOfUninitializedDatasourceDoesNotEmitRemovalMetric` covers the never-initialised case. Both fail on the previous head (`expected: <1> but was: <2>` and `expected: <0> but was: <1>`) and pass now; `BrokerSegmentMetadataCacheTest` is 26/26 locally and checkstyle passes. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
