Обсуждение: Lazy xid assignment V3
I tried sending this with the actual patch attached - this time, to pgsql-pacthes, but it seems that my mail didn't get through again. So it've put up the patch here: http://soc.phlo.org/lazyxidassign.v3.patch ---------------------------------------- Hi Here is an updated version of my patch. This is pretty much equivalent to the version V2, apart from the few minor things Tom criticized. The changes are. .) elog(ERROR) instead of Assert if there are to-be-deleted files, but no XID assigned on commit .) ResourceOwnerId is renamed to VirtualTransactionId. .) GetOldestXmin() checks GetTopTransactionIdIfAny() first, and only reverts to ReadNewTransactionId() if the former returns InvalidTransactionId. Since we didn't really reach an agreement on how xid_age should behave, I've reverted it back to the original version. So with this patch, xid_age will just force assignment of a xid. The other proposed ideas where .) Use some cached xid value which is kept stable for the duration of a transaction. .) Use RecentGlobalXmin. This makes the notion of xid age consistent with VACCUM. The value could change between statements though (for read-committed transactions). .) Use the current snaphot's xmax. This would be the closest possible approximation of using ReadNewTransasctionId() that keeps the xid age constant during a statement or transaction. (Depending on the xact being read-committed or serializable). greetings, Florian Pflug
Florian G. Pflug wrote: > Since we didn't really reach an agreement on how xid_age should behave, > I've reverted it back to the original version. So with this patch, > xid_age will just force assignment of a xid. Is this really a good idea? I'm repeating myself, but a query like select age(xmin) from bigtable could accelerate Xid wraparound. If the server is running close to the limit it could cause a shutdown to prevent the actual wraparound. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On 9/3/07, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Florian G. Pflug wrote: > > Since we didn't really reach an agreement on how xid_age should behave, > > I've reverted it back to the original version. So with this patch, > > xid_age will just force assignment of a xid. > > Is this really a good idea? I'm repeating myself, but a query like > > select age(xmin) from bigtable > > could accelerate Xid wraparound. If the server is running close to the > limit it could cause a shutdown to prevent the actual wraparound. Such query would take only one xid, which should not be a problem? -- marko
Florian G. Pflug wrote: > Here is an updated version of my patch. This is pretty much equivalent > to the version V2, apart from the few minor things Tom criticized. The > changes are. Should there be new a log_line_prefix percent code for virtual transaction ids? Or should we change the meaning of %x to be virtual transaction id instead of the real one. As the patch stands, %x will just print 0 for all transactions with no xid assigned, and the prefix will change when one is assigned. I believe the main use case for %x is to distinguish which messages are coming from which transaction, and the patch as it is makes it mostly useless for that purpose. The function comment in GetLockConflicts should probably be updated; it's not returning a linked list anymore, but an array terminated by an InvalidVirtualTransactionId. What the worst thing that happens if two sessions are assigned the same session id? Should be infrequent, but not impossible if you have some very long-lived connections and other clients connecting/disconnecting all the time. A few comment typos: asigned -> assigned transcation -> transaction > .) elog(ERROR) instead of Assert if there are to-be-deleted files, but > no XID assigned on commit > > .) ResourceOwnerId is renamed to VirtualTransactionId. > > .) GetOldestXmin() checks GetTopTransactionIdIfAny() first, and only > reverts to ReadNewTransactionId() if the former returns > InvalidTransactionId. > > Since we didn't really reach an agreement on how xid_age should behave, > I've reverted it back to the original version. So with this patch, > xid_age will just force assignment of a xid. Sounds OK to me. It's not going to consume any more xids than it does now, and I don't think xid_age is called very often. What's it for anyway? To write scripts to run VACUUM before xid wrap-around? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Marko Kreen escribió: > On 9/3/07, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Florian G. Pflug wrote: > > > Since we didn't really reach an agreement on how xid_age should behave, > > > I've reverted it back to the original version. So with this patch, > > > xid_age will just force assignment of a xid. > > > > Is this really a good idea? I'm repeating myself, but a query like > > > > select age(xmin) from bigtable > > > > could accelerate Xid wraparound. If the server is running close to the > > limit it could cause a shutdown to prevent the actual wraparound. > > Such query would take only one xid, which should not be a problem? My guess is that it would execute age(xid) once per tuple? Even if all the tuples had the same xmin, there's no cache therefore it would consume as many Xids as there are tuples. Am I missing something? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
On 9/3/07, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Marko Kreen escribió: > > On 9/3/07, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > > Florian G. Pflug wrote: > > > > Since we didn't really reach an agreement on how xid_age should behave, > > > > I've reverted it back to the original version. So with this patch, > > > > xid_age will just force assignment of a xid. > > > > > > Is this really a good idea? I'm repeating myself, but a query like > > > > > > select age(xmin) from bigtable > > > > > > could accelerate Xid wraparound. If the server is running close to the > > > limit it could cause a shutdown to prevent the actual wraparound. > > > > Such query would take only one xid, which should not be a problem? > > My guess is that it would execute age(xid) once per tuple? Even if all > the tuples had the same xmin, there's no cache therefore it would > consume as many Xids as there are tuples. > > Am I missing something? First age() assigns CurrentXid, rest reuse it. -- marko
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> Should there be new a log_line_prefix percent code for virtual
> transaction ids? Or should we change the meaning of %x to be virtual
> transaction id instead of the real one.
I think the latter should be sufficient, especially if we also are showing
vxid in pg_locks and pg_stat_activity.
> What the worst thing that happens if two sessions are assigned the same
> session id? Should be infrequent, but not impossible if you have some
> very long-lived connections and other clients connecting/disconnecting
> all the time.
I think this'll be all right. The only operations we actually do on
vxids is equality comparison. Even if you had a session that was 4
billion new-connections old, it should also have a pretty darn high
localvxid counter, and so a new session coming in and starting its
counter at 0 is highly unlikely to conflict. It's not impossible,
I guess, but the odds against seem so high that it's not worth adding
code to prevent it.
>> Since we didn't really reach an agreement on how xid_age should behave,
>> I've reverted it back to the original version. So with this patch,
>> xid_age will just force assignment of a xid.
> Sounds OK to me. It's not going to consume any more xids than it does
> now, and I don't think xid_age is called very often. What's it for
> anyway? To write scripts to run VACUUM before xid wrap-around?
Yeah. I think xid_age is really just a historical curiosity anyway as
of 8.2 --- there's hardly any real use-case for it.
regards, tom lane
Tom Lane wrote: > "Heikki Linnakangas" <heikki@enterprisedb.com> writes: >> Should there be new a log_line_prefix percent code for virtual >> transaction ids? Or should we change the meaning of %x to be virtual >> transaction id instead of the real one. > > I think the latter should be sufficient, especially if we also are showing > vxid in pg_locks and pg_stat_activity. Hm.. Wouldn't that kind of defeat the idea of a log, if you need the output of pg_locks to interpret it? Maybe we should just show both values for %x? Or just the xid if it's set, and the vid otherwise? BTW, my current patch doesn't show the vid in pg_stat_activity. I initially wanted to add because, because I believed that the xid is also shown in pg_stat_activity. But I then realized that the xid actually *isn't* shown, and the all the other values shown in pg_stat_activity are directly pulled out of the stats collector. So it seems quite messy and inconsistent to add the vid. >> What the worst thing that happens if two sessions are assigned the same >> session id? Should be infrequent, but not impossible if you have some >> very long-lived connections and other clients connecting/disconnecting >> all the time. > > I think this'll be all right. The only operations we actually do on > vxids is equality comparison. Even if you had a session that was 4 > billion new-connections old, it should also have a pretty darn high > localvxid counter, and so a new session coming in and starting its > counter at 0 is highly unlikely to conflict. It's not impossible, > I guess, but the odds against seem so high that it's not worth adding > code to prevent it. Even in the case of a conflict, two transactions still wouldn't be running with the same vid. Rather, the second one would block until the first exits, because we hold an ExclusiveLock on the vid. Still, I overlooked the possibility of this, and it does seem a bit ugly. Maybe we ought to use MyBackendId to disambiguate this? All it would take would be another field backendId in VirtualTransactionId. greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes:
> Tom Lane wrote:
>> "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
>>> Should there be new a log_line_prefix percent code for virtual
>>> transaction ids? Or should we change the meaning of %x to be virtual
>>> transaction id instead of the real one.
>>
>> I think the latter should be sufficient, especially if we also are showing
>> vxid in pg_locks and pg_stat_activity.
> Hm.. Wouldn't that kind of defeat the idea of a log, if you need the
> output of pg_locks to interpret it? Maybe we should just show both
> values for %x? Or just the xid if it's set, and the vid otherwise?
Well, how do you interpret xid in the log today, if not by reference
to those views? The last option seems quite unworkable, especially
for CSV-based logs.
I suppose there's no great harm in providing %v as an additional
prefix format code ...
> BTW, my current patch doesn't show the vid in pg_stat_activity.
Hmm, actually it looks like the join key for that is supposed to be PID,
so maybe we needn't do anything to it.
> Even in the case of a conflict, two transactions still wouldn't be
> running with the same vid. Rather, the second one would block until
> the first exits, because we hold an ExclusiveLock on the vid.
It's definitely sufficient then ...
regards, tom lane
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> Tom Lane wrote: >>> "Heikki Linnakangas" <heikki@enterprisedb.com> writes: >>>> Should there be new a log_line_prefix percent code for virtual >>>> transaction ids? Or should we change the meaning of %x to be virtual >>>> transaction id instead of the real one. >>> I think the latter should be sufficient, especially if we also are showing >>> vxid in pg_locks and pg_stat_activity. > >> Hm.. Wouldn't that kind of defeat the idea of a log, if you need the >> output of pg_locks to interpret it? Maybe we should just show both >> values for %x? Or just the xid if it's set, and the vid otherwise? > > Well, how do you interpret xid in the log today, if not by reference > to those views? The last option seems quite unworkable, especially > for CSV-based logs. Someone might match the xid with xids found on disk. If we replace the xid with the vid, than this is impossible unless you also periodically snapshot pg_locks. > I suppose there's no great harm in providing %v as an additional > prefix format code ... Ok, I'll do this. Shouldn't be too hard, it seems... >> BTW, my current patch doesn't show the vid in pg_stat_activity. > > Hmm, actually it looks like the join key for that is supposed to be PID, > so maybe we needn't do anything to it. Yes, that was my reasoning too. Maybe we want to add both xid and vid to pg_stat_activity one day, but since this patch might hit 8.3, lets keep it as minimal as possible... >> Even in the case of a conflict, two transactions still wouldn't be >> running with the same vid. Rather, the second one would block until >> the first exits, because we hold an ExclusiveLock on the vid. > > It's definitely sufficient then ... This implies that connecting and the immediately starting a transaction (which will then get "1" as it's localTransactionId) which you keep open *will* block some other transaction after 4 billion other connects. I'm not saying that this is unacceptable, I just wanted to clearly state the consequences. greetings, Florian Pflug
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> Tom Lane wrote: >>> "Heikki Linnakangas" <heikki@enterprisedb.com> writes: >>>> Should there be new a log_line_prefix percent code for virtual >>>> transaction ids? Or should we change the meaning of %x to be virtual >>>> transaction id instead of the real one. >>> I think the latter should be sufficient, especially if we also are showing >>> vxid in pg_locks and pg_stat_activity. > >> Hm.. Wouldn't that kind of defeat the idea of a log, if you need the >> output of pg_locks to interpret it? Maybe we should just show both >> values for %x? Or just the xid if it's set, and the vid otherwise? > > Well, how do you interpret xid in the log today, if not by reference > to those views? The last option seems quite unworkable, especially > for CSV-based logs. I don't think people usually interpret the xid in logs in any way. It's just a handy unique (unique enough, ignoring xid wraparound) identifier for the transaction that you can use to figure out what each separate transaction is doing. For that purpose, it doesn't matter if it doesn't match the normal on-disk xid, using vid instead of xid works just fine. Hmm. Or is it unique enough after all? Do we reuse session ids after a server restart? For debugging PostgreSQL bugs, though, having the real xid in the logs is really nice. You can then compare the logs against the tuples on the disk. >> Even in the case of a conflict, two transactions still wouldn't be >> running with the same vid. Rather, the second one would block until >> the first exits, because we hold an ExclusiveLock on the vid. > > It's definitely sufficient then ... Yeah. If we did want to do something more, we could acquire the lock on vid conditionally, and use another vid if acquiring the lock fails. But I don't think it's necessary. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: > Tom Lane wrote: >> "Florian G. Pflug" <fgp@phlo.org> writes: >>> Tom Lane wrote: >>>> "Heikki Linnakangas" <heikki@enterprisedb.com> writes: >>>>> Should there be new a log_line_prefix percent code for virtual >>>>> transaction ids? Or should we change the meaning of %x to be virtual >>>>> transaction id instead of the real one. >>>> I think the latter should be sufficient, especially if we also are showing >>>> vxid in pg_locks and pg_stat_activity. >>> Hm.. Wouldn't that kind of defeat the idea of a log, if you need the >>> output of pg_locks to interpret it? Maybe we should just show both >>> values for %x? Or just the xid if it's set, and the vid otherwise? >> Well, how do you interpret xid in the log today, if not by reference >> to those views? The last option seems quite unworkable, especially >> for CSV-based logs. > > I don't think people usually interpret the xid in logs in any way. It's > just a handy unique (unique enough, ignoring xid wraparound) identifier > for the transaction that you can use to figure out what each separate > transaction is doing. For that purpose, it doesn't matter if it doesn't > match the normal on-disk xid, using vid instead of xid works just fine. > > Hmm. Or is it unique enough after all? Do we reuse session ids after a > server restart? Yes - the sessionIs is just a counter in shared memory, so we start "1" after a restart again. > For debugging PostgreSQL bugs, though, having the real xid in the logs > is really nice. You can then compare the logs against the tuples on the > disk. I take this as a vote for having both "%x" and "%v" for xid and virtual xid respectively. greetings, Florian Pflug
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> Yeah. If we did want to do something more, we could acquire the lock on
> vid conditionally, and use another vid if acquiring the lock fails. But
> I don't think it's necessary.
I was thinking more along the lines of looking through the ProcArray at
backend startup to ensure the sessionID we've chosen is unique, and
choosing another one if not. But this would expend cycles with the
ProcArray locked, for something that seems exceedingly unlikely to
happen,
regards, tom lane