Is ParsePrepareRecord dead function

Started by vignesh Cover 6 years ago8 messages
#1vignesh C
vignesh21@gmail.com

Hi,

I could not locate the caller of ParsePrepareRecord function in twophase.c.
Any idea how it gets called?
or
Is it a dead function?

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

#2Robert Haas
robertmhaas@gmail.com
In reply to: vignesh C (#1)
Re: Is ParsePrepareRecord dead function

On Mon, Jul 29, 2019 at 4:10 AM vignesh C <vignesh21@gmail.com> wrote:

I could not locate the caller of ParsePrepareRecord function in twophase.c.
Any idea how it gets called?
or
Is it a dead function?

It looks like it's not only dead, but stillborn. Commit
1eb6d6527aae264b3e0b9c95aa70bb7a594ad1cf introduced it but without
introducing any code that called it, and nothing has changed since
then.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3vignesh C
vignesh21@gmail.com
In reply to: Robert Haas (#2)
1 attachment(s)
Re: Is ParsePrepareRecord dead function

On Mon, Jul 29, 2019 at 8:24 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jul 29, 2019 at 4:10 AM vignesh C <vignesh21@gmail.com> wrote:

I could not locate the caller of ParsePrepareRecord function in twophase.c.
Any idea how it gets called?
or
Is it a dead function?

It looks like it's not only dead, but stillborn. Commit
1eb6d6527aae264b3e0b9c95aa70bb7a594ad1cf introduced it but without
introducing any code that called it, and nothing has changed since
then.

I feel the code can be safely removed.
Patch for the same is attached.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

Attachments:

001_Unused_function_ParsePrepareRecord_removal.patchapplication/octet-stream; name=001_Unused_function_ParsePrepareRecord_removal.patchDownload
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 477709b..e4d9437 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1332,44 +1332,6 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
 	return buf;
 }
 
-/*
- * ParsePrepareRecord
- */
-void
-ParsePrepareRecord(uint8 info, char *xlrec, xl_xact_parsed_prepare *parsed)
-{
-	TwoPhaseFileHeader *hdr;
-	char	   *bufptr;
-
-	hdr = (TwoPhaseFileHeader *) xlrec;
-	bufptr = xlrec + MAXALIGN(sizeof(TwoPhaseFileHeader));
-
-	parsed->origin_lsn = hdr->origin_lsn;
-	parsed->origin_timestamp = hdr->origin_timestamp;
-	parsed->twophase_xid = hdr->xid;
-	parsed->dbId = hdr->database;
-	parsed->nsubxacts = hdr->nsubxacts;
-	parsed->nrels = hdr->ncommitrels;
-	parsed->nabortrels = hdr->nabortrels;
-	parsed->nmsgs = hdr->ninvalmsgs;
-
-	strncpy(parsed->twophase_gid, bufptr, hdr->gidlen);
-	bufptr += MAXALIGN(hdr->gidlen);
-
-	parsed->subxacts = (TransactionId *) bufptr;
-	bufptr += MAXALIGN(hdr->nsubxacts * sizeof(TransactionId));
-
-	parsed->xnodes = (RelFileNode *) bufptr;
-	bufptr += MAXALIGN(hdr->ncommitrels * sizeof(RelFileNode));
-
-	parsed->abortnodes = (RelFileNode *) bufptr;
-	bufptr += MAXALIGN(hdr->nabortrels * sizeof(RelFileNode));
-
-	parsed->msgs = (SharedInvalidationMessage *) bufptr;
-	bufptr += MAXALIGN(hdr->ninvalmsgs * sizeof(SharedInvalidationMessage));
-}
-
-
 
 /*
  * Reads 2PC data from xlog. During checkpoint this data will be moved to
diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h
index b9a531c..1093085 100644
--- a/src/include/access/twophase.h
+++ b/src/include/access/twophase.h
@@ -47,8 +47,6 @@ extern bool StandbyTransactionIdIsPrepared(TransactionId xid);
 
 extern TransactionId PrescanPreparedTransactions(TransactionId **xids_p,
 												 int *nxids_p);
-extern void ParsePrepareRecord(uint8 info, char *xlrec,
-							   xl_xact_parsed_prepare *parsed);
 extern void StandbyRecoverPreparedTransactions(void);
 extern void RecoverPreparedTransactions(void);
 
#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: vignesh C (#3)
Re: Is ParsePrepareRecord dead function

On 2019-Jul-29, vignesh C wrote:

On Mon, Jul 29, 2019 at 8:24 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jul 29, 2019 at 4:10 AM vignesh C <vignesh21@gmail.com> wrote:

I could not locate the caller of ParsePrepareRecord function in twophase.c.
Any idea how it gets called?
or
Is it a dead function?

It looks like it's not only dead, but stillborn. Commit
1eb6d6527aae264b3e0b9c95aa70bb7a594ad1cf introduced it but without
introducing any code that called it, and nothing has changed since
then.

I feel the code can be safely removed.
Patch for the same is attached.

I think there's a patch from Fujii Masao that touches that? Might be
premature to remove it.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Amit Kapila
amit.kapila16@gmail.com
In reply to: Alvaro Herrera (#4)
Re: Is ParsePrepareRecord dead function

On Tue, Jul 30, 2019 at 2:34 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Jul-29, vignesh C wrote:

On Mon, Jul 29, 2019 at 8:24 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jul 29, 2019 at 4:10 AM vignesh C <vignesh21@gmail.com> wrote:

I could not locate the caller of ParsePrepareRecord function in twophase.c.
Any idea how it gets called?
or
Is it a dead function?

It looks like it's not only dead, but stillborn. Commit
1eb6d6527aae264b3e0b9c95aa70bb7a594ad1cf introduced it but without
introducing any code that called it, and nothing has changed since
then.

I feel the code can be safely removed.
Patch for the same is attached.

I think there's a patch from Fujii Masao that touches that? Might be
premature to remove it.

Okay, can you point to that patch? Recently, Robert/Thomas has raised
a comment on undo machinery wherein we are considering to store
FullTransactionId in two-phase file. So, in that connection, we need
to modify this function as well. It is not impossible to test some
unused function (we can try it by superficially calling it at
someplace in code for the purpose of test), but it would have been
better if it is used in someplace.

[1]: /messages/by-id/CA+Tgmob1Oby7Wc5ryB_VBccU9N+uSKjXXocgT9dY_edfxqSA8Q@mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Kapila (#5)
Re: Is ParsePrepareRecord dead function

On 2019-Jul-30, Amit Kapila wrote:

On Tue, Jul 30, 2019 at 2:34 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

I think there's a patch from Fujii Masao that touches that? Might be
premature to remove it.

Okay, can you point to that patch?

/messages/by-id/CAHGQGwFQgRWMOoRfbOOHXy1VdGM-YkwdwvWr_bD0TQXFTjD9Tw@mail.gmail.com

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#7Michael Paquier
michael@paquier.xyz
In reply to: Amit Kapila (#5)
Re: Is ParsePrepareRecord dead function

On Tue, Jul 30, 2019 at 08:42:06AM +0530, Amit Kapila wrote:

Okay, can you point to that patch?

Here you go:
https://commitfest.postgresql.org/23/2105/
The thread is mostly waiting after Fujii-san for an update.
--
Michael

#8vignesh C
vignesh21@gmail.com
In reply to: Alvaro Herrera (#4)
Re: Is ParsePrepareRecord dead function

On Tue, Jul 30, 2019 at 2:34 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2019-Jul-29, vignesh C wrote:

On Mon, Jul 29, 2019 at 8:24 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jul 29, 2019 at 4:10 AM vignesh C <vignesh21@gmail.com> wrote:

I could not locate the caller of ParsePrepareRecord function in twophase.c.
Any idea how it gets called?
or
Is it a dead function?

It looks like it's not only dead, but stillborn. Commit
1eb6d6527aae264b3e0b9c95aa70bb7a594ad1cf introduced it but without
introducing any code that called it, and nothing has changed since
then.

I feel the code can be safely removed.
Patch for the same is attached.

I think there's a patch from Fujii Masao that touches that? Might be
premature to remove it.

Ok, it makes sense not to remove it when there is some work being done in
different thread.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com