ricky2129 commented on PR #10609:
URL: https://github.com/apache/seatunnel/pull/10609#issuecomment-4126821920
@nzw921rx thanks for the contribution, the feature design is good and the
XA/exactly-once guard is the right call.
Three things worth addressing from what i believe:
1. WARN log fires for every JDBC Sink job by default
```
if (batchIntervalMs <= 0 || batchSize == 1) {
LOG.warn("JDBC periodic flush automatically disabled,
batch_interval_ms={}, batch_size={}",
batchIntervalMs, batchSize);
return null;
}
```
When batch_interval_ms = 0 (the default), every JDBC Sink task logs a WARN
on startup even though nothing is wrong — this will spam existing production
job logs after
upgrading. These are two different cases that should be handled
separately:
- batchIntervalMs <= 0 → feature not enabled, no log or LOG.debug at most
- batchIntervalMs > 0 but batchSize = 1 or is_exactly_once = true → WARN
is correct, user explicitly set it but it's being silently ignored
2. Thread name uses caller thread ID, not the scheduler thread ID
` thread.setName("jdbc-batch-flush-scheduler-" +
Thread.currentThread().getId());`
Thread.currentThread() inside the ThreadFactory lambda is the thread that
calls scheduleWithFixedDelay (the open() thread), not the new thread being
created. With
parallelism > 1, multiple instances end up with the same thread name,
making log debugging harder. Should be thread.getId().
3. No lower bound on batch_interval_ms
Setting batch_interval_ms = 1 would cause 1000 scheduler wakeups/second.
Each is a no-op when batchCount = 0 but the lock acquisition overhead adds up
under high
throughput. Worth adding a minimum value check or at least a WARN for
values below a reasonable threshold (e.g. 100ms).
cc @davidzollo for additional suggestions
--
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]
