luozihen commented on PR #12015: URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5613705069
@DanielLeens Thanks for the careful catch on the static cache — that's a fair trade-off to call out. For Issue 1, I’m currently leaning toward the bounded-LRU option rather than the per-job scoping you preferred, but I haven’t finished that change yet. Here’s the reasoning: The connector's JdbcSinkFactory.createSink() is invoked once per table by the engine's per-table loop (MultipleTableJobConfigParser), not once per job, and the TableSinkFactory contract has no per-job initialization hook where I could compile the patterns once and pass the compiled map down through that loop. Doing that would require touching the engine's sink-planning path, which feels outside the scope of this connector PR. So, to keep the change local while still bounding the memory, I’m planning to replace the unbounded ConcurrentHashMap with a small, JDK-only bounded LRU (LinkedHashMap with removeEldestEntry, wrapped in Collections.synchronizedMap, capped at 256 entries). This should keep the memoization benefit (each distinct pattern compiles once and is reused) while putting a hard upper bound on growth, without introducing a new dependency such as Caffeine. I also plan to add a short comment noting that the cache is shared across jobs in this JVM and is bounded. I’ll also note the engine-level IT/E2E as a follow-up in the PR description, as agreed. If you'd still prefer the per-job scoping, I'm happy to follow up separately — I just wanted to avoid expanding the change surface into the engine within this PR. Let me know what you'd prefer. Thanks again! -- 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]
