singhpk234 commented on code in PR #14629:
URL: https://github.com/apache/iceberg/pull/14629#discussion_r2542640993
##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -777,6 +774,9 @@ private static void planFilesFor(TableScan tableScan,
String planId, int tasksPe
private static void asyncPlanFiles(
TableScan tableScan, String asyncPlanId, int tasksPerPlanTask) {
- ASYNC_PLANNING_POOL.execute(() -> planFilesFor(tableScan, asyncPlanId,
tasksPerPlanTask));
+ // Its not necessary to run this in a separate thread pool, but doing so
+ // will create a race condition where a client can call fetchPlanningResult
+ // even before the IN_MEMORY_PLANNING_STATE has been populated.
+ planFilesFor(tableScan, asyncPlanId, tasksPerPlanTask);
Review Comment:
> I guess I don't quite understand this comment:
what i meant is that due to the IN_MEMORY_PLANNING_STATE not being populated
by the time the client comes back to fetchPlanResult, we will get plan-id not
found even when the ASYNC worker is doing its job, let me think i can phrase it
better.
> keep track of "ongoing" and then remove it when it's completed, and the
server handling makes sure it checks those states accordingly when trying to
return the status. Right now, I could see that there's a race
precisely, we would have to implement proper await on the ASYNC worker to
complete before responding, as we expect to respond with the completed status
in the fetchPlanningResult, for simplicity and from client POV choose this
instead, as fake is enough for now, please let me know if you feel otherwise,
happy to take a stab on state management
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
