DanielLeens commented on PR #12328:
URL: https://github.com/apache/seatunnel/pull/12328#issuecomment-5701071888

   @davidzollo Good question, and worth being precise here since the two 
readings of "default" point in opposite directions.
   
   With the documented example (`map: engine*: map-store: enabled: true`), 
`engine_finishedJobMetrics` already inherits `enabled: true` from the `engine*` 
wildcard match, since there was previously no exact-match entry for it. That 
inherited `true` is exactly what causes the unbounded MapStore/WAL growth 
described in #12022 — flipping the *default* to `true` wouldn't change 
anything, because it is already effectively `true` via wildcard inheritance 
today. What's missing is the opt-out, which is what this PR adds.
   
   If instead you meant "should we harden this at the code level instead of 
relying on every doc example getting the override right" — I'd agree that's 
more robust, and I flagged exactly that as a non-blocking follow-up suggestion 
in my review: giving `engine_finishedJobMetrics` the same hardcoded 
`NoOpMapStorage` guard that `FileMapStore.init()` already applies to 
`engine_runningJobMetrics` 
(`seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/persistence/FileMapStore.java:46-53`),
 so correctness wouldn't depend on users copying the right YAML at all.
   
   I'd keep that as a separate follow-up PR rather than folding it into this 
one, though: it's a behavior/data-retention change to production code 
(finished-job metrics would stop persisting even for users who never touch 
their `hazelcast.yaml`), so it deserves its own scoped review and tests. This 
PR is correctly scoped to just fix the documented example so it matches the 
guard `engine_runningJobMetrics` already has in code. Happy to review that 
follow-up if either of you wants to open it.


-- 
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]

Reply via email to