SEZ9 commented on issue #12058: URL: https://github.com/apache/seatunnel/issues/12058#issuecomment-5658064254
Thanks for running the isolated A/B exactly on the agreed contract and reporting it regardless of outcome — that is the evidence I asked for. Reading the artifacts you posted (`fbc0524d4` baseline, gist `66008c5bad44fb05825db40198349175`): BEFORE 173.607 ± 61.793 us/op (CV 32.2%) vs AFTER 180.675 ± 69.126 us/op (CV 34.6%) is a null result. The error bars fully overlap, the per-fork raw values show the same outlier behaviour on both sides (376.0 before, 350.0 / 291.2 after), and the CV did not improve. Combined with your wall/CPU observation that the timed path is still `updateOverview` → `MapProxyImpl.compute` → `AbstractInvocationFuture.get` / `LockSupport.park` on both sides, with `hsync` / `HdfsWriter.flush` at roughly 0.5% of CPU samples, the conclusion I draw for this issue is: - On `file:///`, collapsing the flush path to a single hsync-family call per branch does not move the score or the CV, and it does not remove the original park/`InvocationFuture.get` hotspot. So the storage sync path is not the driver of the variance reported here; the wait is on the invocation-future side. - As agreed, this is filesystem-specific. It says nothing about a real HDFS/DFS client, where the multi-sync + trailing `hflush` shape could still matter for cost, but it also means #12081 cannot claim to address #12058 on this basis. #12081 stays Related only. Two remaining asks to close this out: 1. Your comment is cut off mid-way through the CPU section, and the GC comparison is missing from what I can see here. If the full CPU/GC numbers and the after-side `hsync` percentage are in `AB-REPORT.md`, please just confirm that; otherwise please paste them so the record on this issue is complete. 2. Since the result is null on `file:///`, please state in the report whether you consider the flush-collapse experiment concluded for #12058, or whether you intend to rerun the same single-variable contract against a non-local filesystem. Either answer is fine, but I do not want the local null result to be silently generalised in either direction. Once those two are in, I will treat the flush path as ruled out for this issue and we can keep the investigation focused on the `InvocationFuture.get` / park wait. <!-- streview-comment:1022 --> -- 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]
