Add distributed snapshot support to pg_export_snapshot

[bmdoil]

Add distributed snapshot support to pg_export_snapshot

This PR adds distributed snapshot metadata when exporting and
importing snapshots.

Calling select pg_export_snapshot() from the QD will write the
following fields to the snapshot file in pg_snapshots in the
coordinator data directory.

dsxminall
dsid
dsxmin
dsxmax
dscnt
dsxip

When a snapshot is imported via SET TRANSACTION SNAPSHOT from the QD, subsequent
queries will have the distributed snapshot metadata dispatched to the QEs.
This enables cluster-wide data visibility consistency.

Reference: https://groups.google.com/a/greenplum.org/g/gpdb-dev/c/C6cY8yIbcps

[soumyadeep2007]

I have outlined some key changes that need to be made due to the nature of the master branch and have some comments on the tests.

Also, we should probably disallow export/import of distributed snapshots in utility mode connections (until we ever do gp_export_snapshot) - we can discuss this.

PS: I took the opportunity to do a small refactor that may help during your testing. See attached patch:
0001-Refactor-setDistributedTransactionContext.patch.txt
You can apply it w/ git am. Please order it so that it comes before your 2 commits in this PR.

[jimmyyih]

So we're replacing the local snapshot logic with distributed snapshots for the pg_export_snapshot() catalog function? IMO, it should be a new catalog function like gp_export_distributed_snapshot() to make it more isolated and have the option to backport to 6X as an extension (if that's what we want).

[soumyadeep2007]

So we're replacing the local snapshot logic with distributed snapshots for the pg_export_snapshot() catalog function? IMO, it should be a new catalog function like gp_export_distributed_snapshot() to make it more isolated and have the option to backport to 6X as an extension (if that's what we want).

Not replacing really. The way to think about this is: when we export a snapshot, if it has a distributed snapshot component we export that component too. If it doesn't (like when we are in utility mode), we don't export it. The same reasoning goes behind import. This kind of reflects the whole-part relationship between SnapshotData and DistributedSnapshotWithLocalMapping.

I personally don't like the idea of isolating this to a separate function (when would we want to export just the distributed snapshot and not the other fields in the snapshot, assuming thats what gp_export_distributed_snapshot() will do? If so, then would we need SET TRANSACTION DISTRIBUTED SNAPSHOT and a pg_distributed_snapshot dir?)

Thoughts?

[bmdoil]

So we're replacing the local snapshot logic with distributed snapshots for the pg_export_snapshot() catalog function? IMO, it should be a new catalog function like gp_export_distributed_snapshot() to make it more isolated and have the option to backport to 6X as an extension (if that's what we want).

Not replacing really. The way to think about this is: when we export a snapshot, if it has a distributed snapshot component we export that component too.

For context, the on-disk representation looks like this if we've exported a distributed snapshot.

pid:732997
dbid:13356
iso:2
ro:0
xmin:3456
xmax:3456
xcnt:0
sof:0
sxcnt:0
rec:0
dsxminall:90865
dsid:173
dsxmin:90865
dsxmax:90866
dsxcnt:1
dsxip:90865

Related to catalog implications, should we consider adding transaction information functions for distributed transaction details as well? If so, should they be new i.e. dstxid_current_snapshot() ?

[soumyadeep2007]

Related to catalog implications, should we consider adding transaction information functions for distributed transaction details as well? If so, should they be new i.e. dstxid_current_snapshot() ?

Do you have an immediate use case in mind? IMO lets not introduce it without one.

[ashwinstar]

Opening a utility mode connection, trying to import a snapshot (which has a ds field) and then ensue that we ERROR out. We have to introduce this ERROR in the code. Something like:
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("cannot import distributed snapshot in utility mode"),
errhint("export the snapshot in utility mode")));

I don't think we need to ERROR for this case? It's perfectly fine to use distributed snapshot (if wish too) in utility mode connection. What would go wrong? I thought it can be one of the use-cases for distributed snapshot so that can used for utility mode connections as well if required and available. Though only aspect is how will the snapshot data made available to locally on segments but if exist then fine to use it.

[soumyadeep2007]

Though only aspect is how will the snapshot data made available to locally on segments but if exist then fine to use it.

This is precisely why I thought that we should ERROR out. Currently, we don't have the gp_export_snapshot functionality that dispatches pg_export_snapshot to all segments and creates a snapshot (with the ds) in each segment's datadir. We don't have a current need for it: PXF is resorting to gp2gp I believe for this purpose? Since we don't have a reliable way to export it, if a ds is present in the snapshot being imported into the U connection, we have no way to judge its provenance. Banning will prevent ugly hacks and ensure that if someone needs that functionality they will be nudged to do something in core (gp_export_snapshot).

[ashwinstar]

Though only aspect is how will the snapshot data made available to locally on segments but if exist then fine to use it.

This is precisely why I thought that we should ERROR out. Currently, we don't have the gp_export_snapshot functionality that dispatches pg_export_snapshot to all segments and creates a snapshot (with the ds) in each segment's datadir. We don't have a current need for it: PXF is resorting to gp2gp I believe for this purpose? Since we don't have a reliable way to export it, if a ds is present in the snapshot being imported into the U connection, we have no way to judge its provenance. Banning will prevent ugly hacks and ensure that if someone needs that functionality they will be nudged to do something in core (gp_export_snapshot).

Sounds good. Lets capture that context for banning as comment in code (no real tech reason just mechanics and all this background). So, that years from now if need arises we know the rational for ban/error.

[soumyadeep2007]

LGTM! Just one outstanding comment about the import validation.