Re: Improve LWLock tranche name visibility across backends
От | Sami Imseih |
---|---|
Тема | Re: Improve LWLock tranche name visibility across backends |
Дата | |
Msg-id | CAA5RZ0vw-r1qjHw5hUNTzAyCiK3KQ-r+G5ECfWSn+qjdu-fpnw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Improve LWLock tranche name visibility across backends (Nathan Bossart <nathandbossart@gmail.com>) |
Ответы |
Re: Improve LWLock tranche name visibility across backends
Re: Improve LWLock tranche name visibility across backends |
Список | pgsql-hackers |
> * I modified the array of tranche names to be a char ** to simplify > lookups. I also changed how it is first initialized in CreateLWLocks() a > bit. That works also. > * I've left out the tests for now. Those are great for the development > phase, but I'm not completely sold on committing them. In any case, I'd > probably treat that as a follow-up effort once this stuff is committed. I was mainly using them to verify the functionality since we lack tests in this area. I think long-term, it will be good to have test coverage. We don't need to decide now. > I think this patch set will require reworking the "GetNamedLWLockTranche > crashes on Windows in normal backend" patch [0], but AFAICT we can easily > adjust it to scan through NamedLWLockTrancheNames instead. Except, we will need to turn the char**NamedLWLockTranche into a struct that will hold the num_lwlocks as well. Something like. But that is doable. ``` typedef struct NamedLWLockTranche { char trancheName[NAMEDATALEN]; int num_lwlocks; } NamedLWLockTranche; ``` If there is no interest to backpatch [0], maybe we should just make this change as part of this patch set. What do you think? I can make this change in v18. > Overall, I'm pretty happy about these patches. They simplify the API and > the code while also fixing the problem with tranche name visibility. Just a few things that were discussed earlier, that I incorporated now. 1/ We should be checking that tranche_name is NOT NULL when LWLockNewTrancheId or RequestNamedLWLockTranche is called. Also check for the length of the tranche name inside RequestNamedLWLockTranche, instead of relying on an Assert to check the length. I think that is much safer. 2/ Bertrand suggested to use strlcpy instead of strcpy when copying while holding a spinlock [1] 3/ There was a doc update to mention the 63 byte tranche length. We can skip that for now, since the ERROR message in v16 is clear about the limit. [0] https://postgr.es/m/CAA5RZ0v1_15QPg5Sqd2Qz5rh_qcsyCeHHmRDY89xVHcy2yt5BQ%40mail.gmail.com [1] https://www.postgresql.org/message-id/CAA5RZ0ve-fHuNYW-ruMwg1y1v7-aCqMm_MiNq1KOdg2Y2-pKDw%40mail.gmail.com -- Sami
Вложения
В списке pgsql-hackers по дате отправления: