On Fri, Feb 24, 2023 at 03:16:10PM +0900, Masahiko Sawada wrote: > Thanks! It looks good to me so I've marked it as Ready for Committer. + case 'P': /* Parallel progress reporting */ + { + /* Call the progress reporting callback */ + Assert(pcxt->parallel_progress_callback); + pcxt->parallel_progress_callback(pcxt->parallel_progress_callback_arg); + + break; + }
The key point of the patch is here. From what I understand based on the information of the thread, this is used as a way to make the progress reporting done by the leader more responsive so as we'd update the index counters each time the leader is poked at with a 'P' message by one of its workers, once a worker is done with the parallel cleanup of one of the indexes. That's appealing, because this design is responsive and cheap, while we'd rely on CFIs to make sure that the leader triggers its callback on a timely basis. Unfortunately, sticking a concept of "Parallel progress reporting" is rather confusing here? This stuff can be used for much more purposes than just progress reporting: the leader would execute the callback it has registered based on the timing given by one or more of its workers, these willing to push an event on the leader. Progress reporting is one application of that to force a refresh and make the information of the leader accurate. What about things like a chain of callbacks, for example? Could the leader want to register more than one callback and act on all of them with one single P message? Another question I have: could the reporting of each individual worker make sense on its own? The cleanup of the indexes depends on the order they are processed, their number, size and AM with their cleanup strategy, still there may be a point in getting information about how much work a single worker is doing rather than just have the aggregated information given to the leader? Btw, Is an assertion really helpful here? If parallel_progress_callback is not set, we'd just crash one line after. It seems to me that it could be cleaner to do nothing if a leader gets a poke message from a worker if there are no callbacks registered. -- Michael
signature.asc
Description: PGP signature