From: | "Mikheev, Vadim" <vmikheev(at)SECTORBASE(dot)COM> |
---|---|
To: | "'Tom Lane'" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgreSQL(dot)org |
Subject: | RE: Strangeness in xid allocation / snapshot setup |
Date: | 2001-07-12 02:00:27 |
Message-ID: | 3705826352029646A3E91C53F7189E320166C3@sectorbase2.sectorbase.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> I am trying to understand why GetSnapshotData() needs to acquire the
> SInval spinlock before it calls ReadNewTransactionId, rather than after.
> I see that you made it do so --- in the commit at
>
http://www.ca.postgresql.org/cgi/cvsweb.cgi/pgsql/src/backend/storage/ipc/sh
mem.c.diff?r1=1.41&r2=1.42
> but I don't understand why the loss of concurrency is "necessary".
> Since we are going to treat all xids >= xmax as in-progress anyway,
> what's wrong with reading xmax before we acquire the SInval lock?
AFAIR, I made so to prevent following:
1. Tx Old is running.
2. Tx S reads new transaction ID in GetSnapshotData() and swapped away
before SInval acquired.
3. Tx New gets new transaction ID, makes changes and commits.
4. Tx Old changes some row R changed by Tx New and commits.
5. Tx S gets snapshot data and now sees R changed by *both* Tx Old and
Tx New *but* does not see *other* changes made by Tx New =>
Tx S reads unconsistent data.
---------
As for issue below - I don't remember why I decided that
it's not important and will need in some time to remember.
> Also, it seems to me that in GetNewTransactionId(), it's important
> for MyProc->xid to be set before releasing XidGenLockId, not after.
> Otherwise there is a narrow window for failure:
>
> 1. Process A calls GetNewTransactionId. It allocates an xid of, say,
> 1234, and increments nextXid to 1235. Just after releasing the
> XidGenLock spinlock, but before it can set its MyProc->xid, control
> swaps away from it.
>
> 2. Process B gets to run. It runs GetSnapshotData. It sees nextXid =
> 1235, and it does not see xid = 1234 in any backend's proc->xid.
> Therefore, B will assume xid 1234 has already terminated, when it
> hasn't.
>
> Isn't this broken? The problem would be avoided if
> GetNewTransactionId
> sets MyProc->xid before releasing the spinlock, since then after
> GetSnapshotData has called ReadNewTransactionId, we know that
> all older
> XIDs that are still active are recorded in proc structures.
>
> Comments?
>
> regards, tom lane
>
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2001-07-12 02:32:25 | Re: Strangeness in xid allocation / snapshot setup |
Previous Message | Tatsuo Ishii | 2001-07-12 01:58:18 | Re: docs Japanese translation |