nzw921rx commented on PR #12081:
URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5556939738
Thanks for narrowing the scope following the discussion, adding sync-path
tests, and clarifying what the read-back test verifies. These are helpful
improvements. The production issues identified in this PR are also worth
discussing further.
I’d like to outline the investigation process and distinguish two questions:
whether these changes are reasonable in their own right, and whether they
explain and resolve the high variance reported in #12058. The first can be
evaluated independently; the second still requires investigation to establish
the connection.
This is primarily an investigation task. There is no need to rush into
changes or close the issue through a PR. Reproducing the behavior, explaining
runtime activity, ruling out a possible cause, or sharing difficulties
encountered are all valuable contributions.
We can draw on the performance analysis methodologies collected by Brendan
Gregg and apply them to this case step by step. The following is guidance for
the investigation, not a requirement to complete everything at once or run
every available tool.
1. Clarify the problem and measurement boundaries.
First identify which iterations are slower and whether the variation
mainly occurs within a fork or between forks. High CV describes relative
dispersion; it does not, by itself, establish a production defect.
Also check the timing boundaries around initialization, warmup, writes,
validation, and cleanup. Operations outside the timed interval do not
contribute directly to the Score, but their allocations, background tasks, or
storage activity may still affect subsequent measurements.
2. Try to reproduce the behavior using the original benchmark in a
consistent local environment.
Start with one method and parameter combination, retain the existing
measurement settings, repeat the runs, and keep all results. Look for
differences between normal and slower iterations.
There is no requirement to use GitHub Actions runners or compare local
performance against runner performance. The same local environment can support
reproduction, profiling, hypothesis testing, and controlled before/after
comparisons. Keep the JDK, JVM arguments, data size, storage configuration, and
machine load as consistent as practical.
Different runners can affect Error and CV as well as absolute latency,
so the cross-runner CV reduction currently reported cannot yet be attributed to
the changes. If the behavior cannot be reproduced locally, sharing the
conditions and results is useful, and we can discuss the next step together.
3. Determine where time is spent before choosing tools.
The USE method can help check relevant resources for utilization,
saturation, and errors. Thread-time analysis can then help distinguish
execution time from time waiting for CPU scheduling, locks, I/O, or other
operations.
CPU profiling can help explain execution costs, while Wall profiling,
lock analysis, and JFR can help investigate waiting, GC, safepoints, and thread
activity. Choose tools according to the question. A wide frame in a Wall
profile does not necessarily consume the most CPU or explain the variance.
4. Connect slower iterations to specific JVM and thread behavior.
Compare what happens during normal and slower iterations. If a thread is
parked, determine what it is waiting for and which operation or thread
completes that wait. If GC is suspected, check whether pauses overlap the
slower iterations and can account for the additional time. For storage waits,
distinguish queueing, writing, and synchronization time.
For this PR, it is particularly important to distinguish the local
filesystem path from the real HDFS path. The current benchmark uses file:///,
so please identify the actual output-stream implementation and what the removed
sync call does in that implementation. Duplicate calls in the HDFS branches do
not directly explain variation in the local measurement, and method-call counts
are not equivalent to disk-sync counts.
5. State a specific hypothesis and prediction, then run a controlled
experiment.
Following the scientific method, describe the suspected cause and what
you expect to observe if it is correct, then design an experiment to test that
prediction.
For example, if redundant synchronization is suspected of causing the
variance, predict which waiting time and slower iterations should decrease when
only the redundant call is removed. Keep other conditions unchanged and test
that prediction. If the result does not match, revise or discard the
explanation.
This PR currently combines sync changes, Future completion and timeout
semantics, and WAL exception handling. It would help to evaluate their effects
separately during the investigation so we can identify which change produces
which outcome. Experimental changes do not need to become a formal PR
immediately.
6. Recheck runtime mechanisms alongside the performance results.
In addition to Score, Error, and CV, compare whether the suspected
waiting, contention, or pauses disappear or become substantially smaller, and
whether the original slower iterations decrease accordingly.
Lower mean latency does not necessarily imply lower CV, and lower CV
does not necessarily imply better latency. Interpret the mean, absolute
dispersion, and sample distribution together. If the Score improves but the
suspected mechanism does not change, we cannot yet conclude that the root cause
has been addressed.
Profiling and scored runs can be performed separately, with consistent
collection settings before and after. A profile aggregated over the entire run
can provide direction, but where possible, also examine the time windows
containing slower iterations.
7. Validate correctness independently, then decide on the formal changes.
Performance improvement and correctness need separate validation. Mock
tests for the sync branches can verify the call paths, and read-back tests can
verify visibility, but those guarantees should not be extended into a claim of
verified crash durability.
For the Future and exception-handling changes, useful checks include how
failures reach callers, how the configured timeout applies to batch waits, and
how subsequent requests and WAL recovery behave after a write exception. A
worker being able to continue processing events and a writer remaining safe to
use are separate questions.
If these changes have independent correctness value, they can proceed on
that basis, potentially as separate changes. They do not need a CV improvement
to justify their value. If the original variance remains unexplained, #12058
can remain open, with the PR title, description, root-cause claims, and
issue-closing statement adjusted accordingly.
If the investigation instead identifies a bug in the benchmark itself,
fix it first and establish a new baseline using the corrected benchmark.
Differences between faulty and corrected measurements should not be presented
as a production performance improvement. Subsequent production optimizations
should use the same corrected measurement for both revisions.
As a concrete next step, you could select one of the two original methods,
repeat it locally, and share the runtime conditions, raw results, and initial
observations. That is enough to return to the issue for discussion; there is no
need to wait until you have found the root cause or prepared a complete PR.
Questions about unfamiliar stacks, thread behavior, or experimental results are
welcome, and we can discuss how to narrow the investigation together.
The issues you have identified and the tests you have added can serve as a
foundation for further work. We can progressively establish which changes
address correctness and which affect the original variance, then decide how
best to organize and move them forward.
--
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]
