On Fri, Jan 6, 2023 at 12:59 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > 3. > > > + else if (shmq_res == SHM_MQ_WOULD_BLOCK) > > > + { > > > + /* Replay the changes from the file, if any. */ > > > + if (pa_has_spooled_message_pending()) > > > + { > > > + pa_spooled_messages(); > > > + } > > > > > > I think we do not need this pa_has_spooled_message_pending() function. > > > Because this function is just calling pa_get_fileset_state() which is > > > acquiring mutex and getting filestate then if the filestate is not > > > FS_EMPTY then we call pa_spooled_messages() that will again call > > > pa_get_fileset_state() which will again acquire mutex. I think when > > > the state is FS_SERIALIZE_IN_PROGRESS it will frequently call > > > pa_get_fileset_state() consecutively 2 times, and I think we can > > > easily achieve the same behavior with just one call. > > > > > > > This is just to keep the code easy to follow. As this would be a rare > > case, so thought of giving preference to code clarity. > > I think the code will be simpler with just one function no? I mean > instead of calling pa_has_spooled_message_pending() in if condition > what if we directly call pa_spooled_messages();, this is anyway > fetching the file_state and if the filestate is EMPTY then it can > return false, and if it returns false we can execute the code which is > there in else condition. We might need to change the name of the > function though. > But anyway it is not a performance-critical path so if you think the current way looks cleaner then I am fine with that too.
-- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com