Re: pg_locks display of speculative locks is bogus
Hi, On 2020-02-11 12:24:50 -0800, Peter Geoghegan wrote: > On Tue, Feb 11, 2020 at 12:03 PM Andres Freund wrote: > > Doesn't seem great. > > > > It's trivial to put the xid in the correct place, but it's less obvious > > what to do with the token? For master we should probably add a column, > > but what about the back branches? Ignore it? Put it in classid or such? > > My vote goes to doing nothing about the token on the back branches. > Just prevent bogus pg_locks output. > > Nobody cares about the specifics of the token value -- though perhaps > you foresee a need to have it for testing purposes. I suppose that > adding a column to pg_locks on the master branch is the easy way of > resolving the situation, even if we don't really expect anyone to use > it. You can't really analyze what is waiting for what without seeing it - the prime purpose of pg_locks. So I don't agree with the sentiment that nobody cares about the token. Greetings, Andres Freund
Re: pg_locks display of speculative locks is bogus
On Tue, Feb 11, 2020 at 12:03 PM Andres Freund wrote: > Doesn't seem great. > > It's trivial to put the xid in the correct place, but it's less obvious > what to do with the token? For master we should probably add a column, > but what about the back branches? Ignore it? Put it in classid or such? My vote goes to doing nothing about the token on the back branches. Just prevent bogus pg_locks output. Nobody cares about the specifics of the token value -- though perhaps you foresee a need to have it for testing purposes. I suppose that adding a column to pg_locks on the master branch is the easy way of resolving the situation, even if we don't really expect anyone to use it. -- Peter Geoghegan
pg_locks display of speculative locks is bogus
Hi, I noticed this when tightening up some races for [1] I noticed that the way speculative locks are displayed in pg_locks is completely bogus. As pg_locks has no branch specific to speculative locks, the object etc path is used: case LOCKTAG_OBJECT: case LOCKTAG_USERLOCK: case LOCKTAG_ADVISORY: default:/* treat unknown locktags like OBJECT */ values[1] = ObjectIdGetDatum(instance->locktag.locktag_field1); values[7] = ObjectIdGetDatum(instance->locktag.locktag_field2); values[8] = ObjectIdGetDatum(instance->locktag.locktag_field3); values[9] = Int16GetDatum(instance->locktag.locktag_field4); nulls[2] = true; nulls[3] = true; nulls[4] = true; nulls[5] = true; nulls[6] = true; break; but speculative locks are defined like: /* * ID info for a speculative insert is TRANSACTION info + * its speculative insert counter. */ #define SET_LOCKTAG_SPECULATIVE_INSERTION(locktag,xid,token) \ ((locktag).locktag_field1 = (xid), \ (locktag).locktag_field2 = (token),\ (locktag).locktag_field3 = 0, \ (locktag).locktag_field4 = 0, \ (locktag).locktag_type = LOCKTAG_SPECULATIVE_TOKEN, \ (locktag).locktag_lockmethodid = DEFAULT_LOCKMETHOD) which means that currently a speculative lock's xid is displayed as the database, the token as the classid, and that objid and objsubid are 0 instead of NULL. Doesn't seem great. It's trivial to put the xid in the correct place, but it's less obvious what to do with the token? For master we should probably add a column, but what about the back branches? Ignore it? Put it in classid or such? Greetings, Andres Freund [1] https://www.postgresql.org/message-id/CAAKRu_ZRmxy_OEryfY3G8Zp01ouhgw59_-_Cm8n7LzRH5BAvng%40mail.gmail.com
