Should we add xid_current() or a int8->xid cast?

On Thu, Jul 25, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:

we have txid_current(), which returns an int8. But there's no convenient
way to convert that to type 'xid'. Which is fairly inconvenient, given
that we expose xids in various places.

My current need for this was just a regression test to make sure that
system columns (xmin/xmax in particular) don't get broken again for ON
CONFLICT. But I've needed this before in other scenarios - e.g. age(xid)
can be useful to figure out how old a transaction is, but age() doesn't
work with txid_current()'s return value.

Seems easiest to just add xid_current(), or add a cast from int8 to xid
(probably explicit?) that handles the wraparound logic correctly?

Yeah, I was wondering about that. int8 isn't really the right type,
since FullTransactionId is unsigned. If we had a SQL type for 64 bit
xids, it should be convertible to xid, and the reverse conversion
should require a more complicated dance. Of course we can't casually
change txid_current() without annoying people who are using it, so
perhaps if we invent a new SQL type we should also make a new function
that returns it.

--
Thomas Munro
https://enterprisedb.com

Hi,

On 2019-07-25 12:20:58 +1200, Thomas Munro wrote:

On Thu, Jul 25, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:

we have txid_current(), which returns an int8. But there's no convenient
way to convert that to type 'xid'. Which is fairly inconvenient, given
that we expose xids in various places.

My current need for this was just a regression test to make sure that
system columns (xmin/xmax in particular) don't get broken again for ON
CONFLICT. But I've needed this before in other scenarios - e.g. age(xid)
can be useful to figure out how old a transaction is, but age() doesn't
work with txid_current()'s return value.

Seems easiest to just add xid_current(), or add a cast from int8 to xid
(probably explicit?) that handles the wraparound logic correctly?

Yeah, I was wondering about that. int8 isn't really the right type,
since FullTransactionId is unsigned.

For now that doesn't seem that big an impediment...

If we had a SQL type for 64 bit xids, it should be convertible to xid,
and the reverse conversion should require a more complicated dance.
Of course we can't casually change txid_current() without annoying
people who are using it, so perhaps if we invent a new SQL type we
should also make a new function that returns it.

Possibly we could add a fullxid or xid8, xid64, pg_xid64, ... type, and
have an implicit cast to int8?

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2019-07-25 12:20:58 +1200, Thomas Munro wrote:

On Thu, Jul 25, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:

Seems easiest to just add xid_current(), or add a cast from int8 to xid
(probably explicit?) that handles the wraparound logic correctly?

Yeah, I was wondering about that. int8 isn't really the right type,
since FullTransactionId is unsigned.

For now that doesn't seem that big an impediment...

Yeah, I would absolutely NOT recommend that you open that can of worms
right now. We have looked at adding unsigned integer types in the past
and it looked like a mess.

I think an explicit cast is a reasonable thing to add, though.

regards, tom lane

Hi,

On 2019-07-24 20:34:39 -0400, Tom Lane wrote:

Yeah, I would absolutely NOT recommend that you open that can of worms
right now. We have looked at adding unsigned integer types in the past
and it looked like a mess.

I assume Thomas was thinking more of another bespoke type like xid, just
wider. There's some notational advantage in not being able to
immediately do math etc on xids.

- Andres

Andres Freund <andres@anarazel.de> writes:

On 2019-07-24 20:34:39 -0400, Tom Lane wrote:

Yeah, I would absolutely NOT recommend that you open that can of worms
right now. We have looked at adding unsigned integer types in the past
and it looked like a mess.

I assume Thomas was thinking more of another bespoke type like xid, just
wider. There's some notational advantage in not being able to
immediately do math etc on xids.

Well, we could invent an xid8 type if we want, just don't try to make
it part of the numeric hierarchy (as indeed xid isn't).

regards, tom lane

On Thu, Jul 25, 2019 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

On 2019-07-24 20:34:39 -0400, Tom Lane wrote:

Yeah, I would absolutely NOT recommend that you open that can of worms
right now. We have looked at adding unsigned integer types in the past
and it looked like a mess.

I assume Thomas was thinking more of another bespoke type like xid, just
wider. There's some notational advantage in not being able to
immediately do math etc on xids.

Well, we could invent an xid8 type if we want, just don't try to make
it part of the numeric hierarchy (as indeed xid isn't).

Yeah, I meant an xid64/xid8/fxid/pg_something/... type that isn't a
kind of number.

--
Thomas Munro
https://enterprisedb.com

On Thu, Jul 25, 2019 at 1:11 PM Thomas Munro <thomas.munro@gmail.com> wrote:

On Thu, Jul 25, 2019 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

On 2019-07-24 20:34:39 -0400, Tom Lane wrote:

Yeah, I would absolutely NOT recommend that you open that can of worms
right now. We have looked at adding unsigned integer types in the past
and it looked like a mess.

I assume Thomas was thinking more of another bespoke type like xid, just
wider. There's some notational advantage in not being able to
immediately do math etc on xids.

Well, we could invent an xid8 type if we want, just don't try to make
it part of the numeric hierarchy (as indeed xid isn't).

Yeah, I meant an xid64/xid8/fxid/pg_something/... type that isn't a
kind of number.

I played around with an xid8 type over here (not tested much yet, in
particular not tested on 32 bit box):

/messages/by-id/CA+hUKGKbQtX8E5TEdcZaYhTxqLqrvcpN1Vjb7eCu2bz5EACZbw@mail.gmail.com

--
Thomas Munro
https://enterprisedb.com

On Fri, Aug 2, 2019 at 10:42 PM Thomas Munro <thomas.munro@gmail.com> wrote:

On Thu, Jul 25, 2019 at 1:11 PM Thomas Munro <thomas.munro@gmail.com> wrote:

On Thu, Jul 25, 2019 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

On 2019-07-24 20:34:39 -0400, Tom Lane wrote:

Yeah, I would absolutely NOT recommend that you open that can of worms
right now. We have looked at adding unsigned integer types in the past
and it looked like a mess.

I assume Thomas was thinking more of another bespoke type like xid, just
wider. There's some notational advantage in not being able to
immediately do math etc on xids.

Well, we could invent an xid8 type if we want, just don't try to make
it part of the numeric hierarchy (as indeed xid isn't).

Yeah, I meant an xid64/xid8/fxid/pg_something/... type that isn't a
kind of number.

I thought about how to deal with the transition to xid8 for the
txid_XXX() family of functions. The best idea I've come up with so
far is to create a parallel xid8_XXX() family of functions, and
declare the bigint-based functions to be deprecated, and threaten to
drop them from a future release. The C code for the two families can
be the same (it's a bit of a dirty trick, but only until the
txid_XXX() variants go away). Here's a PoC patch demonstrating that.
Not tested much, yet, probably needs some more work, but I wanted to
see if others thought the idea was terrible first.

I wonder if there is a better way to share hash functions than the
hack in check_hash_func_signature(), which I had to extend to cover
xid8.

Adding to CF.

--
Thomas Munro
https://enterprisedb.com

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com> wrote:

Adding to CF.

Rebased. An OID clashed so re-roll the dice. Also spotted a typo.

--
Thomas Munro
https://enterprisedb.com

Thomas Munro <thomas.munro@gmail.com> writes:

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com> wrote:

Adding to CF.

Rebased. An OID clashed so re-roll the dice. Also spotted a typo.

FWIW, I'd move *all* the OIDs added by this patch up to >= 8000.
I don't feel a strong need to fill in the gaps in the low-numbered
OIDs, and people who do try that are likely to hit problems of the
sort you just did.

regards, tom lane

Thomas Munro <thomas.munro@gmail.com> writes:

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
wrote:

Adding to CF.

Rebased. An OID clashed so re-roll the dice. Also spotted a typo.

I have some questions in this code.

First,
"FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" of
the previous code. "FullTransactionIdPrecedes(xmax, val)" expresses
"val > xmax". Is it all right?

@@ -384,15 +324,17 @@ parse_snapshot(const char *str)
  	while (*str != '\0')
  	{
  		/* read next value */
-		val = str2txid(str, &endp);
+		val = FullTransactionIdFromU64(pg_strtouint64(str, &endp, 10));
  		str = endp;

  		/* require the input to be in order */
-		if (val < xmin || val >= xmax || val < last_val)
+		if (FullTransactionIdPrecedes(val, xmin) ||
+			FullTransactionIdPrecedes(xmax, val) ||
+			FullTransactionIdPrecedes(val, last_val))

In addition to it, as to current TransactionId(not FullTransactionId)
comparison, when we express ">=" of TransactionId, we use
"TransactionIdFollowsOrEquals". this method is referred by some methods.
On the other hand, FullTransactionIdFollowsOrEquals has not implemented
yet. So, how about implementing this method?

Second,
About naming rule, "8" of xid8 means 8 bytes, but "8" has different
meaning in each situation. For example, int8 of PostgreSQL means 8
bytes, int8 of C language means 8 bits. If 64 is used, it just means 64
bits. how about xid64()?

regards,

Takao Fujii

On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com> wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
wrote:

Adding to CF.

Rebased. An OID clashed so re-roll the dice. Also spotted a typo.

I have some questions in this code.

Thanks for looking at the patch.

First,
"FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" of
the previous code. "FullTransactionIdPrecedes(xmax, val)" expresses
"val > xmax". Is it all right?

@@ -384,15 +324,17 @@ parse_snapshot(const char *str)
while (*str != '\0')
{
/* read next value */
-               val = str2txid(str, &endp);
+               val = FullTransactionIdFromU64(pg_strtouint64(str, &endp, 10));
str = endp;

/* require the input to be in order */
-               if (val < xmin || val >= xmax || val < last_val)
+               if (FullTransactionIdPrecedes(val, xmin) ||
+                       FullTransactionIdPrecedes(xmax, val) ||
+                       FullTransactionIdPrecedes(val, last_val))

In addition to it, as to current TransactionId(not FullTransactionId)
comparison, when we express ">=" of TransactionId, we use
"TransactionIdFollowsOrEquals". this method is referred by some methods.
On the other hand, FullTransactionIdFollowsOrEquals has not implemented
yet. So, how about implementing this method?

Good idea. I added the missing variants:

+#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
+#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
+#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)

Second,
About naming rule, "8" of xid8 means 8 bytes, but "8" has different
meaning in each situation. For example, int8 of PostgreSQL means 8
bytes, int8 of C language means 8 bits. If 64 is used, it just means 64
bits. how about xid64()?

In C, the typenames use bits, by happy coincidence similar to the C99
stdint.h typenames (int32_t etc) that we should perhaps eventually
switch to.

In SQL, the types have names based on the number of bytes: int2, int4,
int8, float4, float8, not conforming to any standard but established
over 3 decades ago and also understood by a few other SQL systems.

That's unfortunate, but I can't see that ever changing. I thought
that it would make most sense for the SQL type to be called xid8,
though admittedly it doesn't quite fit the pattern because xid is not
called xid4. There is another example a bit like that: macaddr (6
bytes) and macaccdr8 (8 bytes). As for the C type, we use
TransactionId and FullTransactionId (rather than, say, xid32 and
xid64).

In the attached I also took Tom's advice and used unused_oids script
to pick random OIDs >= 8000 for all new objects (ignoring nearby
comments about the range of OIDs used in different sections of the
file).

On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com>
wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
wrote:

Adding to CF.

Rebased. An OID clashed so re-roll the dice. Also spotted a typo.

I have some questions in this code.

Thanks for looking at the patch.

First,
"FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax"
of
the previous code. "FullTransactionIdPrecedes(xmax, val)" expresses
"val > xmax". Is it all right?

@@ -384,15 +324,17 @@ parse_snapshot(const char *str)
while (*str != '\0')
{
/* read next value */
-               val = str2txid(str, &endp);
+               val = FullTransactionIdFromU64(pg_strtouint64(str, 
&endp, 10));
str = endp;

/* require the input to be in order */
-               if (val < xmin || val >= xmax || val < last_val)
+               if (FullTransactionIdPrecedes(val, xmin) ||
+                       FullTransactionIdPrecedes(xmax, val) ||
+                       FullTransactionIdPrecedes(val, last_val))

In addition to it, as to current TransactionId(not FullTransactionId)
comparison, when we express ">=" of TransactionId, we use
"TransactionIdFollowsOrEquals". this method is referred by some
methods.
On the other hand, FullTransactionIdFollowsOrEquals has not
implemented
yet. So, how about implementing this method?

Good idea. I added the missing variants:

+#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= 
(b).value)
+#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
+#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= 
(b).value)

Thank you for your patch.
It looks good.

Second,
About naming rule, "8" of xid8 means 8 bytes, but "8" has different
meaning in each situation. For example, int8 of PostgreSQL means 8
bytes, int8 of C language means 8 bits. If 64 is used, it just means
64
bits. how about xid64()?

In C, the typenames use bits, by happy coincidence similar to the C99
stdint.h typenames (int32_t etc) that we should perhaps eventually
switch to.

In SQL, the types have names based on the number of bytes: int2, int4,
int8, float4, float8, not conforming to any standard but established
over 3 decades ago and also understood by a few other SQL systems.

That's unfortunate, but I can't see that ever changing. I thought
that it would make most sense for the SQL type to be called xid8,
though admittedly it doesn't quite fit the pattern because xid is not
called xid4. There is another example a bit like that: macaddr (6
bytes) and macaccdr8 (8 bytes). As for the C type, we use
TransactionId and FullTransactionId (rather than, say, xid32 and
xid64).

That makes sense.

Anyway,
In the pg_proc.dat, "xid_snapshot_xip" should be "xid8_snapshot_xip".
And some parts of 0002 patch are rejected when I patch 0002 after
patching 0001.

regards

Hi Thomas,

Please let me ask something about wraparound problem.

+static FullTransactionId
+convert_xid(TransactionId xid, FullTransactionId next_fxid)
 {
-	uint64		epoch;
+	TransactionId next_xid = XidFromFullTransactionId(next_fxid);
+	uint32 epoch = EpochFromFullTransactionId(next_fxid);

...

-	/* xid can be on either side when near wrap-around */
-	epoch = (uint64) state->epoch;
-	if (xid > state->last_xid &&
-		TransactionIdPrecedes(xid, state->last_xid))
+	if (xid > next_xid)
 		epoch--;
-	else if (xid < state->last_xid &&
-			 TransactionIdFollows(xid, state->last_xid))
-		epoch++;

-	return (epoch << 32) | xid;
+	return FullTransactionIdFromEpochAndXid(epoch, xid);

ISTM codes for wraparound are deleted. Is that correct?
I couldn't have read all related threads about using FullTransactionId but
does using FullTransactionId avoid wraparound problem?

If we consider below conditions, we can say it's difficult to see wraparound
with current disk like SSD (2GB/s) or memory DDR4 (34GB/s), but if we can use
more high-spec hardware like HBM3 (2048GB/s), we can see wraparound. Or do
I say silly things?

* 10 year system ( < 2^4 )
* 1 year = 31536000 ( = 60 * 60 * 24 * 365) secs ( < 2^25 )
* 2^35 ( = 2^64 / 2^4 / 2^25) transactions we can use in each seconds
* we can write at (2^5 * 2^30 * n) bytes/sec = (32 * n) GB/sec if we use 'n'
bytes for each transactions.

Is there any agreement we can throw the wraparound problem away if we adopt
FullTransactionId?

Thanks
--
Yoshikazu Imai

On Wed, Nov 20, 2019 at 5:43 PM imai.yoshikazu@fujitsu.com
<imai.yoshikazu@fujitsu.com> wrote:

Is there any agreement we can throw the wraparound problem away if we adopt
FullTransactionId?

Here is one argument for why 64 bits ought to be enough: we use 64 bit
LSNs for the WAL, and it usually takes more than one byte of WAL to
consume a transaction. If you write about 500MB of WAL per second,
your system will break in about a thousand years due to LSN
wraparound, that is, assuming the earth hasn't been destroyed to make
way for a hyperspace bypass, but either way you will probably still
have some spare full transaction IDs.

That's fun to think about, but unfortunately it's not easy to figure
out how to retrofit FullTransactionId into enough places to make
wraparounds go away in the traditional heap. It's a goal of at least
a couple of ongoing new AM projects to not have that problem, and I
figured it was a good idea to lay down very basic facilities for that,
trivial as they might be, and see where else they can be useful...

On 11/3/19 2:43 PM, Thomas Munro wrote:

On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com> wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
wrote:

Adding to CF.

Rebased. An OID clashed so re-roll the dice. Also spotted a typo.

I have some questions in this code.

Thanks for looking at the patch.

First,
"FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" of
the previous code. "FullTransactionIdPrecedes(xmax, val)" expresses
"val > xmax". Is it all right?

@@ -384,15 +324,17 @@ parse_snapshot(const char *str)
while (*str != '\0')
{
/* read next value */
-               val = str2txid(str, &endp);
+               val = FullTransactionIdFromU64(pg_strtouint64(str, &endp, 10));
str = endp;

/* require the input to be in order */
-               if (val < xmin || val >= xmax || val < last_val)
+               if (FullTransactionIdPrecedes(val, xmin) ||
+                       FullTransactionIdPrecedes(xmax, val) ||
+                       FullTransactionIdPrecedes(val, last_val))

In addition to it, as to current TransactionId(not FullTransactionId)
comparison, when we express ">=" of TransactionId, we use
"TransactionIdFollowsOrEquals". this method is referred by some methods.
On the other hand, FullTransactionIdFollowsOrEquals has not implemented
yet. So, how about implementing this method?

Good idea. I added the missing variants:

+#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
+#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
+#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)

Second,
About naming rule, "8" of xid8 means 8 bytes, but "8" has different
meaning in each situation. For example, int8 of PostgreSQL means 8
bytes, int8 of C language means 8 bits. If 64 is used, it just means 64
bits. how about xid64()?

In C, the typenames use bits, by happy coincidence similar to the C99
stdint.h typenames (int32_t etc) that we should perhaps eventually
switch to.

In SQL, the types have names based on the number of bytes: int2, int4,
int8, float4, float8, not conforming to any standard but established
over 3 decades ago and also understood by a few other SQL systems.

That's unfortunate, but I can't see that ever changing. I thought
that it would make most sense for the SQL type to be called xid8,
though admittedly it doesn't quite fit the pattern because xid is not
called xid4. There is another example a bit like that: macaddr (6
bytes) and macaccdr8 (8 bytes). As for the C type, we use
TransactionId and FullTransactionId (rather than, say, xid32 and
xid64).

In the attached I also took Tom's advice and used unused_oids script
to pick random OIDs >= 8000 for all new objects (ignoring nearby
comments about the range of OIDs used in different sections of the
file).

These two patches (v3) no longer apply cleanly. Could you please
rebase?

--
Mark Dilger

On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:

These two patches (v3) no longer apply cleanly. Could you please
rebase?

Hi Mark,
Thanks. Here's v4.

On 11/30/19 5:14 PM, Thomas Munro wrote:

On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:

These two patches (v3) no longer apply cleanly. Could you please
rebase?

Hi Mark,
Thanks. Here's v4.

Thanks, Thomas.

The new patches apply cleanly and pass 'installcheck'.

--
Mark Dilger

On Tue, Dec 3, 2019 at 6:56 AM Mark Dilger <hornschnorter@gmail.com> wrote:

On 11/30/19 5:14 PM, Thomas Munro wrote:

On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:

These two patches (v3) no longer apply cleanly. Could you please
rebase?

Hi Mark,
Thanks. Here's v4.

Thanks, Thomas.

The new patches apply cleanly and pass 'installcheck'.

I rebased, fixed the "xid_snapshot_xip" problem spotted by Takao Fujii
that I had missed earlier, updated a couple of error messages to refer
to the new names (even when using the old functions) and ran
check-world and some simple manual tests on an -m32 build just to be
paranoid. Here are the versions of these patches I'd like to commit.
Does anyone want to object to the txid/xid8 type punning scheme or
long term txid-sunsetting plan?