Best2Two commented on PR #10497: URL: https://github.com/apache/seatunnel/pull/10497#issuecomment-3939083615
@davidzollo Thank you for the detailed and constructive feedback! I really appreciate the time you took to review this thoroughly. I'll address all points systematically: **1. Multi-Table Routing Semantics:** ✅ Will add - Adding documentation comments to clarify that we intentionally support mixed-table rows in one writer instance - This handles edge cases where transforms may route multiple tables to the same writer - The fallback to config table ensures backward compatibility **2. Synchronization Scope in flush():** ✅ Will fix immediately - Excellent catch on the performance issue! - Will move network I/O and sleep outside synchronized block - Lock will only protect the copy + clear operations **3. Retry Policy Hardcoded:** ✅ Will make configurable - Will add `retry.max_attempts` (default: 3) and `retry.base_delay_ms` (default: 100) options - Will update connector options, config, and factory - Will document in connector docs **4. Test Focus:** ✅ Acknowledged - Agree that current tests are implementation-focused - Will keep interface/behavior tests - Can refactor to reduce reflection dependency in a follow-up if needed I'll push these changes within the next hours. Thanks again for the thorough review! -- 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]
