CodingCat commented on PR #2366: URL: https://github.com/apache/celeborn/pull/2366#issuecomment-2055554655
@waitinfuture for the issue you mentioned here https://github.com/apache/celeborn/pull/2366#discussion_r1564412991, I think no matter whether we use while loop to wait or signal, we won't be able to control the capacity precisely due to the scenario you mentioned I gave it a second thought, I feel we probably should go back to the earliest version of this PR to achieve the precise control , i.e. (1) move inbox.post (https://github.com/apache/celeborn/pull/2366/files#diff-3bbc409588c06897cf69e21164917997b058dac53de6e59f4e78f6a8c391883cR172-R173) outside of synchronized block (2) using LinkedBlockingQueue.put() or my current waitOnFull inside .post() to address your original concerns (1) moving inbox.post() call outside of synchronized will make it possible to post some messages to the inbox when the dispatcher already stopped (comments link at https://github.com/apache/celeborn/pull/2366#discussion_r1522632501): I think we should be fine if we make the whole stop() method as synchronized, because dispatcher.stop() will post a Poison to MessageLoop which stopped any further processing of the message (2) using LinkedBlockingQueue.put() or my current waitOnFull inside .post() will lead to a thread holding the synchronized lock while waiting (comments link https://github.com/apache/celeborn/pull/2366#discussion_r1541182027_ ...I feel it is probably fine, since the wait method implemented in LinkedBlockingQueue is not really a busy wait, and if we can move inbox.post() call outside of synchronized block (as suggested by (1) ) , we won't even have this problem what do you think -- 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]
