Обсуждение: Question on LWLockMode in dsa.c
Hi, While working on radix tree patch[1], John Naylor found that dsa.c doesn't already use shared locks even in dsa_dump(). dsa_dump() seems a pure read-only function so I thought we could use a shared lock mode there. Is there any reason to use exclusive mode even in dsa_dump()? Ultimately, since we're trying to add a new function dsa_get_total_size() that just returns dsa_area_control.total_segment_size and therefore would also be a read-only function, I'd like to find out the correct lock mode there. Regards, [1] https://www.postgresql.org/message-id/CANWCAZYYzoKp_4%2B1m5mn-TRD62BTwom8iLXLOWMsHkkwFi%3Drzg%40mail.gmail.com -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Tue, Jan 30, 2024 at 6:24 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > Hi, > > While working on radix tree patch[1], John Naylor found that dsa.c > doesn't already use shared locks even in dsa_dump(). dsa_dump() seems > a pure read-only function so I thought we could use a shared lock mode > there. Is there any reason to use exclusive mode even in dsa_dump()? check_for_freed_segments_locked which is called from dsa_dump does mark a few variables of the dsa area to NULL right? Are you suggesting to take initially a shared lock in dsa_dump and upgrade it to exclusive mode in the unlikely event of unlikely(area->freed_segment_counter != freed_segment_counter) occurring in check_for_freed_segments_locked? > Ultimately, since we're trying to add a new function > dsa_get_total_size() that just returns > dsa_area_control.total_segment_size and therefore would also be a > read-only function, I'd like to find out the correct lock mode there. > > [1] https://www.postgresql.org/message-id/CANWCAZYYzoKp_4%2B1m5mn-TRD62BTwom8iLXLOWMsHkkwFi%3Drzg%40mail.gmail.com Do you see any lock contention or any other issues with exclusive locks on dsa areas? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On Tue, Jan 30, 2024 at 4:43 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Tue, Jan 30, 2024 at 6:24 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > Hi, > > > > While working on radix tree patch[1], John Naylor found that dsa.c > > doesn't already use shared locks even in dsa_dump(). dsa_dump() seems > > a pure read-only function so I thought we could use a shared lock mode > > there. Is there any reason to use exclusive mode even in dsa_dump()? > > check_for_freed_segments_locked which is called from dsa_dump does > mark a few variables of the dsa area to NULL right? Right. But IIUC these are backend-local variables, and we don't change any shared variables. > Are you suggesting > to take initially a shared lock in dsa_dump and upgrade it to > exclusive mode in the unlikely event of > unlikely(area->freed_segment_counter != freed_segment_counter) > occurring in check_for_freed_segments_locked? I thought we could just take a shared lock in dsa_dump(). > > > Ultimately, since we're trying to add a new function > > dsa_get_total_size() that just returns > > dsa_area_control.total_segment_size and therefore would also be a > > read-only function, I'd like to find out the correct lock mode there. > > > > [1] https://www.postgresql.org/message-id/CANWCAZYYzoKp_4%2B1m5mn-TRD62BTwom8iLXLOWMsHkkwFi%3Drzg%40mail.gmail.com > > Do you see any lock contention or any other issues with exclusive > locks on dsa areas? Not particularly for now. But we don't need to unnecessarily take an exclusive lock and even a shared lock looks safe at a glance. So I need to convince myself that we need to take an exclusive lock there in order to add a new dsa_get_total_size() function. And we might need comments. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Tue, Jan 30, 2024 at 6:24 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > Hi, > > While working on radix tree patch[1], John Naylor found that dsa.c > doesn't already use shared locks even in dsa_dump(). dsa_dump() seems > a pure read-only function so I thought we could use a shared lock mode > there. Is there any reason to use exclusive mode even in dsa_dump()? > > Ultimately, since we're trying to add a new function > dsa_get_total_size() that just returns > dsa_area_control.total_segment_size and therefore would also be a > read-only function, I'd like to find out the correct lock mode there. > Doesn't seem like there is any reason for this to be an exclusive lock. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com