_RollbackFunc : dead code?

Started by Alvaro Herreraabout 21 years ago6 messageshackers
Jump to latest
#1Alvaro Herrera
alvherre@dcc.uchile.cl

While trying to grok heap_update I came again across the
_heap_unlock_tuple function. This code apparently tries to save a XLog
round while trying to mark a tuple for update, by registering a
"rollback callback", which would unmark the tuple in case the
transaction is rolled back.

Turns out the callback is never called at all. So the code is dead
code.

Also, it claims that by marking t_infomask with a special
HEAP_XMAX_UNLOGGED bit, this trick would not suffer across a system
crash, because tqual routines would check this bit (comments in the code
don't specify what would the routines do with it).

Turns out tqual routines never check the bit. In fact, the bit is never
checked at all, nowhere in the code: heap_update sets it and then
unsets it, but that's all.

So I think this is dead code. The attached patch removes it.

--
Alvaro Herrera (<alvherre[@]dcc.uchile.cl>)
Maybe there's lots of data loss but the records of data loss are also lost.
(Lincoln Yeoh)

Attachments:

unlogged-update.patchtext/plain; charset=us-asciiDownload+21-96
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#1)
Re: _RollbackFunc : dead code?

Alvaro Herrera <alvherre@dcc.uchile.cl> writes:

So I think this is dead code. The attached patch removes it.

Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
proceed in that direction, so we may as well remove the last traces.

regards, tom lane

#3Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#2)
Re: _RollbackFunc : dead code?

On Sun, 2005-03-27 at 16:01 -0500, Tom Lane wrote:

Alvaro Herrera <alvherre@dcc.uchile.cl> writes:

So I think this is dead code. The attached patch removes it.

Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
proceed in that direction, so we may as well remove the last traces.

Agreed.

We still need to explain *why* at some point, but thats still one of my
WIPs.

Best Regards, Simon Riggs

#4Alvaro Herrera
alvherre@dcc.uchile.cl
In reply to: Simon Riggs (#3)
Re: _RollbackFunc : dead code?

On Thu, Mar 31, 2005 at 05:07:39PM +0100, Simon Riggs wrote:

On Sun, 2005-03-27 at 16:01 -0500, Tom Lane wrote:

Alvaro Herrera <alvherre@dcc.uchile.cl> writes:

So I think this is dead code. The attached patch removes it.

Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
proceed in that direction, so we may as well remove the last traces.

Agreed.

We still need to explain *why* at some point, but thats still one of my
WIPs.

Sorry, what's your WIP? Explain why nobody wants to implement UNDO? Or
implement UNDO? Or why at some point somebody wanted to implement UNDO?

Now I remember that in the WAL docs there is a paragraph or two
mentioning that in a future project we want to implement UNDO ... maybe
it's a good idea to rip that off.

--
Alvaro Herrera (<alvherre[@]dcc.uchile.cl>)
"Un poeta es un mundo encerrado en un hombre" (Victor Hugo)

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#4)
Re: _RollbackFunc : dead code?

Alvaro Herrera <alvherre@dcc.uchile.cl> writes:

Now I remember that in the WAL docs there is a paragraph or two
mentioning that in a future project we want to implement UNDO ... maybe
it's a good idea to rip that off.

It is gone as of 8.0.

regards, tom lane

#6Simon Riggs
simon@2ndQuadrant.com
In reply to: Alvaro Herrera (#4)
Re: _RollbackFunc : dead code?

On Thu, 2005-03-31 at 12:24 -0400, Alvaro Herrera wrote:

On Thu, Mar 31, 2005 at 05:07:39PM +0100, Simon Riggs wrote:

On Sun, 2005-03-27 at 16:01 -0500, Tom Lane wrote:

Alvaro Herrera <alvherre@dcc.uchile.cl> writes:

So I think this is dead code. The attached patch removes it.

Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
proceed in that direction, so we may as well remove the last traces.

Agreed.

We still need to explain *why* at some point, but thats still one of my
WIPs.

Sorry, what's your WIP? Explain why nobody wants to implement UNDO? Or
implement UNDO? Or why at some point somebody wanted to implement UNDO?

Nearly: explain why UNDO is not required, and need not be implemented.

Anyway, its a WIP, but not unfortunately a high one, right now.

Now I remember that in the WAL docs there is a paragraph or two
mentioning that in a future project we want to implement UNDO ... maybe
it's a good idea to rip that off.

I agree.

Best Regards, Simon Riggs