Copilot commented on code in PR #12462:
URL: https://github.com/apache/seatunnel/pull/12462#discussion_r4090480196
##########
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/source/enumerator/splitter/AbstractJdbcSourceChunkSplitterTest.java:
##########
@@ -155,6 +155,61 @@ public void testEfficientShardingThroughSampling() throws
NoSuchMethodException
ChunkRange.of(6, null)));
}
+ /**
+ * When IDs are non-consecutive (distributionFactor > 1.0) and sampling is
enabled, the
+ * splitEvenlySizedChunks path should NOT be used. Instead, sampling-based
splitting should be
+ * preferred for better chunk balance.
+ *
+ * <p>This test verifies that for a table with IDs spanning a wide range
but few actual rows
+ * (distributionFactor >> 1.0), the algorithm falls back to arithmetic
stepping only when
+ * sampling is disabled.
+ */
+ @Test
+ public void testSplitEvenlySizedChunksWithNonConsecutiveIds() {
Review Comment:
This test does not exercise the code added in the production class: it calls
`splitEvenlySizedChunks` and `efficientShardingThroughSampling` directly, while
the new conditional lives in the private `splitTableIntoChunks` flow and
depends on `JdbcSourceConfig`, min/max, row-count, and the sampling callback.
It would still pass if the new conditional were removed entirely. Add a test
that drives the split-table path with stubbed query/sample results and verifies
that sampling is selected (and the fallback path when sampling is empty).
##########
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/main/java/org/apache/seatunnel/connectors/cdc/base/source/enumerator/splitter/AbstractJdbcSourceChunkSplitter.java:
##########
@@ -173,6 +173,46 @@ private List<ChunkRange> splitTableIntoChunks(
&& doubleCompare(distributionFactor,
distributionFactorUpper) <= 0;
if (dataIsEvenlyDistributed) {
+ // When distributionFactor > 1.0, primary key IDs are not
consecutive
+ // (there are gaps in the ID space). Arithmetic-based
evenly-sized chunk
+ // splitting assumes gaps are uniformly distributed, which
fails when
+ // gaps are clustered (e.g., large ranges of deleted IDs). In
this case,
+ // prefer sampling-based splitting which uses actual data
points for
+ // chunk boundaries, producing balanced splits regardless of
gap
+ // distribution.
+ int shardCount = (int) (approximateRowCnt / chunkSize);
+ if (doubleCompare(distributionFactor, 1.0d) > 0
+ && sampleShardingAllow
+ && shardCount > 0) {
Review Comment:
This condition bypasses the existing `sampleShardingThreshold` and makes
every evenly-distributed table with any ID gap and more than one chunk perform
the sampling query. For MySQL, `sampleDataFromColumn` scans the entire table
before retaining samples
(`connector-cdc-mysql/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/mysql/utils/MySqlUtils.java:142-192`),
so even a small two- or ten-shard table now incurs an additional full-table
read and sort. Please retain a size/threshold guard (or add an explicit opt-in
for this more expensive path) rather than triggering it solely on
`distributionFactor > 1.0`.
--
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]
