Copilot commented on code in PR #2623:
URL: https://github.com/apache/phoenix/pull/2623#discussion_r4214318254


##########
phoenix-core-server/src/main/java/org/apache/phoenix/mapreduce/PhoenixInputFormat.java:
##########
@@ -82,6 +94,34 @@ public RecordReader<NullWritable, T> 
createRecordReader(InputSplit split,
 
   @Override
   public List<InputSplit> getSplits(JobContext context) throws IOException, 
InterruptedException {
+    boolean coalescingEnabled = 
context.getConfiguration().getBoolean(SPLIT_COALESCING_ENABLED,
+      DEFAULT_SPLIT_COALESCING_ENABLED);
+    if (coalescingEnabled) {
+      // Coalescing merges all of a RegionServer's splits into a single 
mapper, so stats-based
+      // splitting (which fragments each region into many sub-scan splits to 
gain parallelism) would
+      // just be undone here, wasting the guidepost work. Force 
region-granular splits before
+      // coalescing, mirroring PhoenixSyncTableTool.
+      if 
(PhoenixConfigurationUtil.getSplitByStats(context.getConfiguration())) {
+        LOGGER.info(
+          "Split coalescing is enabled ({}); disabling stats-based splitting 
({}) so "
+            + "region-granular splits are coalesced per RegionServer",
+          SPLIT_COALESCING_ENABLED, 
PhoenixConfigurationUtil.MAPREDUCE_SPLIT_BY_STATS);
+      }
+      PhoenixConfigurationUtil.setSplitByStats(context.getConfiguration(), 
false);
+    }
+    List<InputSplit> baseSplits = getBaseSplits(context);
+    return coalescingEnabled
+      ? RegionServerSplitCoalescer.coalesceWithGuard(baseSplits)
+      : baseSplits;

Review Comment:
   Coalescing makes one mapper consume many scans serially, but 
`PhoenixRecordReader.initialize` assigns every `TableResultIterator` the same 
deadline up front (`PhoenixRecordReader.java:132-155`), while 
`ConcatResultIterator` opens later scanners only after earlier scans finish. A 
later scan can therefore begin with an already-expired deadline and fail on a 
heartbeat before its first valid row (especially after leading empty/slow 
regions), whereas the original per-region mapper would receive a fresh timeout. 
Please establish the timeout when each scanner is actually opened, or otherwise 
preserve a usable per-scan timeout for coalesced splits.



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