Re: [PR] [CELEBORN-2332] Fix self join deadlock in C++ WorkerPartitionReader fetch callbacks [celeborn]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #3693:
URL: https://github.com/apache/celeborn/pull/3693#discussion_r3308582666


##########
cpp/celeborn/client/reader/WorkerPartitionReader.cpp:
##########
@@ -122,6 +136,8 @@ void WorkerPartitionReader::initAndCheck() {
       shared_this->chunkQueue_.enqueue(std::move(chunk));
       VLOG(1) << "WorkerPartitionReader::onSuccess: "
               << streamChunkSlice.toString();
+      destructionExecutor().add(
+          [s = std::move(shared_this)]() mutable { s.reset(); });
     };

Review Comment:
   `destructionExecutor().add(...)` is executed on every successful chunk fetch, which posts a task per chunk even though the off-thread handoff is only needed when this callback holds the last `shared_ptr` reference. This can add noticeable scheduling overhead and keep the reader alive longer than necessary under heavy fetch workloads. Consider only offloading when `shared_this.use_count() == 1` (or an equivalent "last ref" check), and otherwise let `shared_this` fall out of scope normally.



##########
cpp/celeborn/client/reader/WorkerPartitionReader.cpp:
##########
@@ -138,6 +154,8 @@ void WorkerPartitionReader::initAndCheck() {
         auto exp = shared_this->exception_.wlock();
         *exp = std::move(exception);
       }
+      destructionExecutor().add(
+          [s = std::move(shared_this)]() mutable { s.reset(); });
     };

Review Comment:
   `destructionExecutor().add(...)` is invoked for every failure callback, which will enqueue an additional task even when there are still other `shared_ptr` owners of the reader. Since fetch failures can also be frequent (e.g., transient network errors), consider guarding the off-thread handoff so it only runs when this callback is about to drop the last `shared_ptr` reference, to avoid unnecessary executor load.



-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org