viirya commented on PR #58090: URL: https://github.com/apache/spark/pull/58090#issuecomment-5605385026
Overall, the shared harness is well structured, and the consistent source-time warm-up filter and offset draining make the latency comparison more robust. Explicitly distinguishing window aggregation's sample populations and omitting potentially misleading percentile ratios is also a good choice. One possible follow-up: could we include the updated count in the `transformWithState` output and validate it per key? Currently, the processor updates `countState` but emits the input row unchanged, and validation only checks the output row count. If state were unexpectedly reset across batch boundaries, the benchmark could still pass and report latency numbers despite exercising incorrect state behavior. Emitting the updated count would let us verify both the per-key count sequence and the final count against the input. The existing workload already revisits keys across RTM batch boundaries, so this should fit into the current run. -- 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]
