Core: Make REST scan planning poll retries and backoff configurable by Bartekszost · Pull Request #17846 · apache/iceberg

Bartekszost · 2026-08-27T12:58:21Z

Summary

Make the full REST async scan-planning poll policy configurable in RESTTableScan, so deployments that support longer-running server-side scan plans can raise the previous hardcoded limits (see also Core : Make REST scan planning poll timeout configurable #15863)
rest-scan-planning.poll-num-retries bounds extra poll attempts after the first fetch (default remains 10; 0 means a single fetch with no retries)
rest-scan-planning.poll-min-wait-ms (default 1s), rest-scan-planning.poll-max-wait-ms (default 1 min), and rest-scan-planning.poll-scale-factor (default 2.0) control exponential backoff between polls
Validate retries as non-negative, wait times as positive with min <= max, and scale factor as >= 1.0
Include the configured timeout and retry limit in the RemotePlanTimeoutException message
Document the poll properties (including the existing rest-scan-planning.poll-timeout-ms) in catalog properties

Test plan

./gradlew :iceberg-core:test --tests org.apache.iceberg.rest.TestRESTScanPlanning — passed (existing cases plus the new ones below; 0 failures)

asyncPlanningRespectsConfigurablePollRetries: poll-num-retries=0 against a server that never completes; poll loop stops after one fetch
asyncPlanningSucceedsWithCustomRetries: non-default poll-num-retries=25; async planning completes
asyncPlanningRejectsInvalidRetries: negative retries rejected with IllegalArgumentException
asyncPlanningSucceedsWithCustomBackoff: non-default min wait, max wait, and scale factor; async planning completes
asyncPlanningRejectsInvalidPollBackoff: invalid min wait, max wait, and scale factor rejected
asyncPlanningRejectsMinWaitGreaterThanMaxWait: min wait > max wait rejected

AI Disclosure

Model: Cursor Grok 4.6
Platform/Tool: Cursor
Human Oversight: partially reviewed
Prompt Summary: Make REST scan planning poll retries and backoff (min wait, max wait, scale factor) configurable catalog properties, add unit tests, and update catalog property docs.

Allow catalogs to set rest-scan-planning.poll-num-retries so long-running remote plans are not stopped by the default retry count. Generated-by: Cursor

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds a new REST catalog property to make async REST scan planning poll retry count configurable, replacing a previously hardcoded retry limit and expanding tests around async planning behavior.

Changes:

Introduce rest-scan-planning.poll-num-retries (default 10) and validate it is non-negative.
Use the configured retry value in RESTTableScan polling and include it in timeout diagnostics.
Add unit tests covering configured retry behavior, success path, and invalid configuration.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java	Adds new catalog property constants for configurable poll retry count.
core/src/main/java/org/apache/iceberg/rest/RESTTableScan.java	Reads/validates the new property, uses it in the poll loop, and improves timeout messaging.
core/src/test/java/org/apache/iceberg/rest/TestRESTScanPlanning.java	Adds tests for honoring retry config, custom retries, and rejecting invalid values.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Bartekszost · 2026-08-28T10:57:58Z

+    catalogWithAdapter.catalog.initialize(
+        "test-custom-retries",
+        ImmutableMap.of(
+            CatalogProperties.FILE_IO_IMPL,
+            "org.apache.iceberg.inmemory.InMemoryFileIO",
+            RESTCatalogProperties.SCAN_PLANNING_MODE,
+            RESTCatalogProperties.ScanPlanningMode.SERVER.modeName(),
+            RESTCatalogProperties.REST_SCAN_PLANNING_POLL_NUM_RETRIES,
+            "10"));
+
+    RESTTable table = restTableFor(catalogWithAdapter.catalog, "custom_retries_success");
+    setParserContext(table);
+    assertThat(table.newScan().planFiles()).hasSize(1);


Good catch. The success path was using the default (10), so it would not fail if the property were ignored. Updated it to a non-default value (25), matching the timeout test which uses 30000 rather than the 5-minute default.

+    // With 0 retries and a server that never completes, planFiles should fail after one attempt
+    assertThatThrownBy(scan::planFiles)
+        .isInstanceOf(RemotePlanTimeoutException.class)
+        .hasMessageContaining("did not complete within configured limits");


Bartekszost · 2026-08-28T10:57:57Z

      "rest-scan-planning.poll-timeout-ms";
  public static final long REST_SCAN_PLANNING_POLL_TIMEOUT_MS_DEFAULT =
      TimeUnit.MINUTES.toMillis(5);



Added a short comment on the constant: this is extra poll attempts after the first fetch, must be >= 0, and 0 means a single fetch with no retries.

dillitz · 2026-08-28T10:00:14Z

  private static final long MIN_SLEEP_MS = 1000; // Initial delay
  private static final long MAX_SLEEP_MS = 60 * 1000; // Max backoff delay (1 minute)
-  private static final int MAX_RETRIES = 10; // Max number of poll retries
  private static final double SCALE_FACTOR = 2.0; // Exponential scale factor


Thinking further, at this point, wouldn't it make sense to make the other 3 retry parameters configurable as well? 😄

Good call — the remaining backoff knobs are now catalog properties as well (poll-min-wait-ms, poll-max-wait-ms, poll-scale-factor), with the same defaults as the old constants, validation, tests, and docs.

Expose min wait, max wait, and scale factor alongside poll retries so long-running remote plans can tune the full backoff policy, not only the retry count. Generated-by: Cursor

dillitz

LGTM

uros-b · 2026-08-28T11:42:20Z

+    // With 0 retries and a server that never completes, planFiles should fail after one attempt
+    assertThatThrownBy(scan::planFiles)
+        .isInstanceOf(RemotePlanTimeoutException.class)
+        .hasMessageContaining("did not complete within configured limits");


The assertion .hasMessageContaining("did not complete within configured limits") does not verify that the configured retry count appears in the RemotePlanTimeoutException message. The PR specifically advertises that the configured value is surfaced in the exception for diagnosability (format: "(timeout=%d ms, maxRetries=%d)"); with poll-num-retries=0 configured, adding .hasMessageContaining("maxRetries=0") would guard against regression in that interpolation.

Good catch, asyncPlanningRespectsConfigurablePollRetries now also asserts maxRetries=0 in the RemotePlanTimeoutException message.

Lock the RemotePlanTimeoutException message to include maxRetries so a dropped interpolation would fail the test. Generated-by: Cursor

Core: Make REST scan planning poll retries configurable

ce5c2a7

Allow catalogs to set rest-scan-planning.poll-num-retries so long-running remote plans are not stopped by the default retry count. Generated-by: Cursor

Copilot AI lite review requested due to automatic review settings August 27, 2026 12:58

github-actions Bot added the core label Aug 27, 2026

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Copilot started reviewing on behalf of Bartekszost August 27, 2026 13:22 View session

dillitz reviewed Aug 28, 2026

View reviewed changes

Core: Make REST scan planning poll retries and backoff configurable

20ffb99

Expose min wait, max wait, and scale factor alongside poll retries so long-running remote plans can tune the full backoff policy, not only the retry count. Generated-by: Cursor

github-actions Bot added the docs label Aug 28, 2026

Bartekszost changed the title ~~Core: Make REST scan planning poll retries configurable~~ Core: Make REST scan planning poll retries and backoff configurable Aug 28, 2026

dillitz approved these changes Aug 28, 2026

View reviewed changes

uros-b reviewed Aug 28, 2026

View reviewed changes

Core: Assert configured poll retry count in timeout exception

b2fe291

Lock the RemotePlanTimeoutException message to include maxRetries so a dropped interpolation would fail the test. Generated-by: Cursor

Navigation Menu

Uh oh!

Core: Make REST scan planning poll retries and backoff configurable - #17846

Core: Make REST scan planning poll retries and backoff configurable#17846
Bartekszost wants to merge 3 commits into
apache:mainfrom
Bartekszost:core-configurable-scan-poll-retries

Bartekszost commented Aug 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Bartekszost Aug 28, 2026

Uh oh!

Bartekszost Aug 28, 2026

Uh oh!

dillitz Aug 28, 2026

Uh oh!

Bartekszost Aug 28, 2026

Uh oh!

dillitz left a comment

Uh oh!

uros-b Aug 28, 2026

Uh oh!

Bartekszost Aug 28, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Bartekszost commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Bartekszost Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Bartekszost Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

dillitz Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Bartekszost Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

dillitz left a comment

Choose a reason for hiding this comment

Uh oh!

uros-b Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Bartekszost Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Bartekszost commented Aug 27, 2026 •

edited

Loading

Bartekszost Aug 28, 2026 •

edited

Loading