Обсуждение: Weird behavior with "sensitive" cursors.
Hi -
I'm seeing a behavior with updatable cursors that matches neither the behavior
of a sensitive cursor nor an insensitive one. In summary, I'm running with
serializable as the isolation level and rows updated within the same
transaction seem to disappear under the cursor.
From the postgres documentation (I'm using 8.3.0), specifying FOR UPDATE should
provide the client with a sensitive cursor: "If the cursor's query includes
FOR UPDATE or FOR SHARE, then returned rows are locked at the time they are
first fetched, in the same way as for a regular SELECT command with these
options. In addition, the returned rows will be the most up-to-date versions;
therefore these options provide the equivalent of what the SQL standard calls a
sensitive cursor."
But then I get this behavior:
{{{
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SET
begin;
BEGIN
show transaction isolation level;
transaction_isolation
-----------------------
serializable
(1 row)
create table foo (a bigint);
CREATE TABLE
insert into foo select generate_series(0, 9);
INSERT 0 10
select * from foo;
a
---
0
1
2
3
4
5
6
7
8
9
(10 rows)
declare c1 no scroll cursor for select * from foo for update;
DECLARE CURSOR
update foo set a=1000 where a>5;
UPDATE 4
fetch all from c1;
a
---
0
1
2
3
4
5
(6 rows)
select * from foo;
a
------
0
1
2
3
4
5
1000
1000
1000
1000
(10 rows)
abort;
ROLLBACK
}}}
Based on my interpretation of cursor sensitivity, I should:
* See rows 0 through 9 if the cursor is insensitive. In fact, this is what I
get if I remove the FOR UPDATE option.
* See the same as a SELECT command executed within the same transaction if the
cursor is sensitive.
This seems like a bug to me, and it prevents one from getting sensitive cursors
with postgres. Can anybody explain the behavior above?
thanks a lot,
-daniel
Daniel F escribió: > SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SERIALIZABLE; > begin; > show transaction isolation level; > create table foo (a bigint); > insert into foo select generate_series(0, 9); > declare c1 no scroll cursor for select * from foo for update; > update foo set a=1000 where a>5; > fetch all from c1; Interesting. If I create an non-unique index on the table before declaring the cursor, FETCH throws an error: alvherre=# fetch all from c1; ERROR: attempted to lock invisible tuple -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Interesting. If I create an non-unique index on the table before
> declaring the cursor, FETCH throws an error:
> alvherre=# fetch all from c1;
> ERROR: attempted to lock invisible tuple
I get that in 8.4 and HEAD even without any index, just trying the given
case. It looks to me like this is a bug in the new snapshot management.
The cursor is using CurrentSnapshot directly --- it does not have a
private copy --- and therefore when CommandCounterIncrement happens
it affects what the cursor can "see". The cursor should not be able
to "see" any tuples created after it was created.
I think we need to ensure that when a cursor is created, it obtains a
private copy of the current snapshot ... but I'm not sure where that
ought to happen. Thoughts?
regards, tom lane
Tom Lane escribió: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Interesting. If I create an non-unique index on the table before > > declaring the cursor, FETCH throws an error: > > > alvherre=# fetch all from c1; > > ERROR: attempted to lock invisible tuple > > I get that in 8.4 and HEAD even without any index, just trying the given > case. It looks to me like this is a bug in the new snapshot management. > The cursor is using CurrentSnapshot directly --- it does not have a > private copy --- and therefore when CommandCounterIncrement happens > it affects what the cursor can "see". The cursor should not be able > to "see" any tuples created after it was created. Right, but what if it tries to grab a tuple that was updated? When the portal tries to fetch the tuple, it has become "invisible", thus the error, no? > I think we need to ensure that when a cursor is created, it obtains a > private copy of the current snapshot ... but I'm not sure where that > ought to happen. Thoughts? Maybe you are right, but I don't think that's the only bug here. I think the right place is PerformCursorOpen; instead of having PortalStart use GetActiveSnapshot(), it should create a copy of it. The first problem is that we don't have any API in snapmgr.c for this "gimme a private copy of this snap"; I think we'll have to open up CopySnapshot to outside calls :-( I played a bit with doing this only when the OPT_CURSOR_INSENSITIVE bit is set, but I'm not ever seeing it set -- with or with FOR UPDATE ... -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera escribió: > I played a bit with doing this only when the OPT_CURSOR_INSENSITIVE bit > is set, but I'm not ever seeing it set -- with or with FOR UPDATE ... Oh, I see, that's a grammar only bit. I need to check rowMarks == NIL instead. It doesn't help anyway but at least I figured that bit out :-( -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane escribi�:
>> I think we need to ensure that when a cursor is created, it obtains a
>> private copy of the current snapshot ... but I'm not sure where that
>> ought to happen. Thoughts?
> Maybe you are right, but I don't think that's the only bug here.
Well, the first problem is that 8.4 is failing to duplicate the
historical behavior. After that we can think about whether we'd like to
change the historical behavior. I'm not entirely convinced about that
yet (and it certainly wouldn't be something I'd want to back-patch).
regards, tom lane
Tom Lane escribió: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Tom Lane escribi�: > >> I think we need to ensure that when a cursor is created, it obtains a > >> private copy of the current snapshot ... but I'm not sure where that > >> ought to happen. Thoughts? > > > Maybe you are right, but I don't think that's the only bug here. > > Well, the first problem is that 8.4 is failing to duplicate the > historical behavior. Oh! That's easy. > After that we can think about whether we'd like to > change the historical behavior. I'm not entirely convinced about that > yet (and it certainly wouldn't be something I'd want to back-patch). I don't care enough about that to spend much time on it ... -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Вложения
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane escribió:
>> Well, the first problem is that 8.4 is failing to duplicate the
>> historical behavior.
> Oh! That's easy.
I don't think that testing rowMarks is the right thing at all here.
That tells you whether it's a SELECT FOR UPDATE, but actually we
want any cursor (and only cursors) to have a private snapshot.
Also, do we really need the Register bit? Won't the portal register
its use of the snapshot anyway (or if it doesn't, isn't that a bug)?
regards, tom lane
Tom Lane escribió: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Tom Lane escribió: > >> Well, the first problem is that 8.4 is failing to duplicate the > >> historical behavior. > > > Oh! That's easy. > > I don't think that testing rowMarks is the right thing at all here. > That tells you whether it's a SELECT FOR UPDATE, but actually we > want any cursor (and only cursors) to have a private snapshot. Hmm, okay. > Also, do we really need the Register bit? Won't the portal register > its use of the snapshot anyway (or if it doesn't, isn't that a bug)? Perhaps, but registering it a second time does not harm, and I think it's cleaner to expose the new RegisterCopiedSnapshot function than bare CopySnapshot. In PortalStart there's something I'd like to clean up in HEAD, which is that we're setting up the snapshot as Active only to be able to do GetActive to pass it down to CreateQueryDesc. That's silly -- we should just get a local snap to pass down; no need to push, get, pop. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Tom Lane escribió: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Tom Lane escribió: > >> Well, the first problem is that 8.4 is failing to duplicate the > >> historical behavior. > > > Oh! That's easy. > > I don't think that testing rowMarks is the right thing at all here. > That tells you whether it's a SELECT FOR UPDATE, but actually we > want any cursor (and only cursors) to have a private snapshot. The attached patch implements this. I intend to apply to 8.4 and HEAD shortly. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Вложения
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane escribi�:
>> I don't think that testing rowMarks is the right thing at all here.
>> That tells you whether it's a SELECT FOR UPDATE, but actually we
>> want any cursor (and only cursors) to have a private snapshot.
> The attached patch implements this. I intend to apply to 8.4 and HEAD
> shortly.
Looks sane. Can we add a short regression test sequence that checks
for this?
regards, tom lane
Tom Lane escribió: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Tom Lane escribi�: > >> I don't think that testing rowMarks is the right thing at all here. > >> That tells you whether it's a SELECT FOR UPDATE, but actually we > >> want any cursor (and only cursors) to have a private snapshot. > > > The attached patch implements this. I intend to apply to 8.4 and HEAD > > shortly. > > Looks sane. Can we add a short regression test sequence that checks > for this? Something is wrong with the patch :-( I'm getting WARNING: Snapshot reference leak: Snapshot 0x1be5840 still referenced with a simple test case. Still investigating. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
This one really works and includes a basic test case. You were right that the extra Register was bogus :-( I had to expose CopySnapshot, which I still don't like but ... (I could have added an extra Unregister somewhere during portal close, but it would have meant making everything messier). -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Вложения
Alvaro Herrera escribió: > > This one really works and includes a basic test case. You were right > that the extra Register was bogus :-( I had to expose CopySnapshot, > which I still don't like but ... (I could have added an extra > Unregister somewhere during portal close, but it would have meant making > everything messier). Applied (to HEAD and 8.4). -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.