public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 03/10] Make sure published XIDs are persistent
5+ messages / 4 participants
[nested] [flat]

* [PATCH 03/10] Make sure published XIDs are persistent
@ 2021-03-08 06:43  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: Kyotaro Horiguchi @ 2021-03-08 06:43 UTC (permalink / raw)

pg_xact_status() premises that XIDs obtained by
pg_current_xact_id(_if_assigned)() are persistent beyond a crash. But
XIDs are not guaranteed to go beyond WAL buffers before commit and
thus XIDs may vanish if server crashes before commit. This patch
guarantees the XID shown by the functions to be flushed out to disk.

Copied from: https://www.postgresql.org/message-id/20210308.173242.463790587797836129.horikyota.ntt%40gmail.com
---
 src/backend/access/transam/xact.c | 55 +++++++++++++++++++++++++------
 src/backend/access/transam/xlog.c |  2 +-
 src/backend/utils/adt/xid8funcs.c | 12 ++++++-
 src/include/access/xact.h         |  3 +-
 4 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 6395a9b240..38e978d238 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -201,7 +201,7 @@ typedef struct TransactionStateData
 	int			prevSecContext; /* previous SecurityRestrictionContext */
 	bool		prevXactReadOnly;	/* entry-time xact r/o state */
 	bool		startedInRecovery;	/* did we start in recovery? */
-	bool		didLogXid;		/* has xid been included in WAL record? */
+	XLogRecPtr 	minLSN;			/* LSN needed to reach to record the xid */
 	int			parallelModeLevel;	/* Enter/ExitParallelMode counter */
 	bool		chain;			/* start a new block after this one */
 	bool		assigned;		/* assigned to top-level XID */
@@ -520,14 +520,46 @@ GetCurrentFullTransactionIdIfAny(void)
  *	MarkCurrentTransactionIdLoggedIfAny
  *
  * Remember that the current xid - if it is assigned - now has been wal logged.
+ *
+ * upto is the LSN up to which we need to flush WAL to ensure the current xid
+ * to be persistent. See EnsureCurrentTransactionIdLogged().
  */
 void
-MarkCurrentTransactionIdLoggedIfAny(void)
+MarkCurrentTransactionIdLoggedIfAny(XLogRecPtr upto)
 {
-	if (FullTransactionIdIsValid(CurrentTransactionState->fullTransactionId))
-		CurrentTransactionState->didLogXid = true;
+	if (FullTransactionIdIsValid(CurrentTransactionState->fullTransactionId) &&
+		XLogRecPtrIsInvalid(CurrentTransactionState->minLSN))
+		CurrentTransactionState->minLSN = upto;
 }
 
+/*
+ *	EnsureCurrentTransactionIdLogged
+ *
+ * Make sure that the current top XID is WAL-logged.
+ */
+void
+EnsureTopTransactionIdLogged(void)
+{
+	/*
+	 * We need at least one WAL record for the current top transaction to be
+	 * flushed out.  Write one if we don't have one yet.
+	 */
+	if (XLogRecPtrIsInvalid(TopTransactionStateData.minLSN))
+	{
+		xl_xact_assignment xlrec;
+
+		xlrec.xtop = XidFromFullTransactionId(XactTopFullTransactionId);
+		Assert(TransactionIdIsValid(xlrec.xtop));
+		xlrec.nsubxacts = 0;
+
+		XLogBeginInsert();
+		XLogRegisterData((char *) &xlrec, MinSizeOfXactAssignment);
+		TopTransactionStateData.minLSN =
+			XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT);
+	}
+
+	XLogFlush(TopTransactionStateData.minLSN);
+}
 
 /*
  *	GetStableLatestTransactionId
@@ -616,14 +648,14 @@ AssignTransactionId(TransactionState s)
 	 * When wal_level=logical, guarantee that a subtransaction's xid can only
 	 * be seen in the WAL stream if its toplevel xid has been logged before.
 	 * If necessary we log an xact_assignment record with fewer than
-	 * PGPROC_MAX_CACHED_SUBXIDS. Note that it is fine if didLogXid isn't set
+	 * PGPROC_MAX_CACHED_SUBXIDS. Note that it is fine if minLSN isn't set
 	 * for a transaction even though it appears in a WAL record, we just might
 	 * superfluously log something. That can happen when an xid is included
 	 * somewhere inside a wal record, but not in XLogRecord->xl_xid, like in
 	 * xl_standby_locks.
 	 */
 	if (isSubXact && XLogLogicalInfoActive() &&
-		!TopTransactionStateData.didLogXid)
+		XLogRecPtrIsInvalid(TopTransactionStateData.minLSN))
 		log_unknown_top = true;
 
 	/*
@@ -693,6 +725,7 @@ AssignTransactionId(TransactionState s)
 			log_unknown_top)
 		{
 			xl_xact_assignment xlrec;
+			XLogRecPtr		   endptr;
 
 			/*
 			 * xtop is always set by now because we recurse up transaction
@@ -707,11 +740,13 @@ AssignTransactionId(TransactionState s)
 			XLogRegisterData((char *) unreportedXids,
 							 nUnreportedXids * sizeof(TransactionId));
 
-			(void) XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT);
+			endptr = XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT);
 
 			nUnreportedXids = 0;
-			/* mark top, not current xact as having been logged */
-			TopTransactionStateData.didLogXid = true;
+
+			/* set minLSN of top, not of current xact if not yet */
+			if (XLogRecPtrIsInvalid(TopTransactionStateData.minLSN))
+				TopTransactionStateData.minLSN = endptr;
 		}
 	}
 }
@@ -2022,7 +2057,7 @@ StartTransaction(void)
 	 * initialize reported xid accounting
 	 */
 	nUnreportedXids = 0;
-	s->didLogXid = false;
+	s->minLSN = InvalidXLogRecPtr;
 
 	/*
 	 * must initialize resource-management stuff first
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 15da91a8dd..6eb46ea8a7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1172,7 +1172,7 @@ XLogInsertRecord(XLogRecData *rdata,
 	 */
 	WALInsertLockRelease();
 
-	MarkCurrentTransactionIdLoggedIfAny();
+	MarkCurrentTransactionIdLoggedIfAny(EndPos);
 
 	END_CRIT_SECTION();
 
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index cc2b4ac797..992482f8c8 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -357,6 +357,8 @@ bad_format:
 Datum
 pg_current_xact_id(PG_FUNCTION_ARGS)
 {
+	FullTransactionId xid;
+
 	/*
 	 * Must prevent during recovery because if an xid is not assigned we try
 	 * to assign one, which would fail. Programs already rely on this function
@@ -365,7 +367,12 @@ pg_current_xact_id(PG_FUNCTION_ARGS)
 	 */
 	PreventCommandDuringRecovery("pg_current_xact_id()");
 
-	PG_RETURN_FULLTRANSACTIONID(GetTopFullTransactionId());
+	xid = GetTopFullTransactionId();
+
+	/* the XID is going to be published, make sure it is psersistent */
+	EnsureTopTransactionIdLogged();
+
+	PG_RETURN_FULLTRANSACTIONID(xid);
 }
 
 /*
@@ -380,6 +387,9 @@ pg_current_xact_id_if_assigned(PG_FUNCTION_ARGS)
 	if (!FullTransactionIdIsValid(topfxid))
 		PG_RETURN_NULL();
 
+	/* the XID is going to be published, make sure it is psersistent */
+	EnsureTopTransactionIdLogged();
+
 	PG_RETURN_FULLTRANSACTIONID(topfxid);
 }
 
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 34cfaf542c..a61e4d6da5 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -386,7 +386,8 @@ extern FullTransactionId GetTopFullTransactionId(void);
 extern FullTransactionId GetTopFullTransactionIdIfAny(void);
 extern FullTransactionId GetCurrentFullTransactionId(void);
 extern FullTransactionId GetCurrentFullTransactionIdIfAny(void);
-extern void MarkCurrentTransactionIdLoggedIfAny(void);
+extern void MarkCurrentTransactionIdLoggedIfAny(XLogRecPtr upto);
+extern void EnsureTopTransactionIdLogged(void);
 extern bool SubTransactionIsActive(SubTransactionId subxid);
 extern CommandId GetCurrentCommandId(bool used);
 extern void SetParallelStartTimestamps(TimestampTz xact_ts, TimestampTz stmt_ts);
-- 
2.17.0


--jozmn01XJZjDjM3N
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0004-wal_compression_method-default-to-zlib.patch"



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Is OpenSSL AES-NI not available in pgcrypto?
@ 2023-01-02 16:57  [email protected] <[email protected]>
  0 siblings, 2 replies; 5+ messages in thread

From: [email protected] @ 2023-01-02 16:57 UTC (permalink / raw)
  To: [email protected]

Hi all,

A question, may I wrong.

I've a Rocky Linux 8 with OpenSSL 1.1.1 FIPS  and Intel cpu with aes 
support (cat /proc/cpuinfo | grep aes)

Test made with openssl gives me a huge performance with aes enabled vs not:

"openssl speed -elapsed -evp aes-128-cbc" is about 5 time faster than 
"openssl speed -elapsed aes-128-cbc" or another "software calculated 
test", eg. "openssl speed -elapsed bf-cbc"

So OpenSSL is ok.

Postgresql 15 is compiled with openssl:

select name, setting from pg_settings where name = 'ssl_library';
     name     | setting
-------------+---------
  ssl_library | OpenSSL
(1 row)

So, a test with pgcrypto:

select pgp_sym_encrypt(data::text, 'pwd') --default to aes128
from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, 
'1 hour'::interval) data

vs

select pgp_sym_encrypt(data::text, 'pwd','cipher-algo=bf') -- blowfish
from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, 
'1 hour'::interval) data

In my test both queries execution is similar....aes-128 was expected 
about  5 time faster.

So, why?

Pgcrypto use OpenSSL as backend, so, does it explicit force software aes 
calculation instead of AES-NI cpu ones?

Thanksfor support.

Best regards,

Agharta










^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Is OpenSSL AES-NI not available in pgcrypto?
@ 2023-01-03 15:54  Peter Eisentraut <[email protected]>
  parent: [email protected] <[email protected]>
  1 sibling, 1 reply; 5+ messages in thread

From: Peter Eisentraut @ 2023-01-03 15:54 UTC (permalink / raw)
  To: [email protected] <[email protected]>; [email protected]

On 02.01.23 17:57, [email protected] wrote:
> select pgp_sym_encrypt(data::text, 'pwd') --default to aes128
> from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, 
> '1 hour'::interval) data
> 
> vs
> 
> select pgp_sym_encrypt(data::text, 'pwd','cipher-algo=bf') -- blowfish
> from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, 
> '1 hour'::interval) data
> 
> In my test both queries execution is similar....aes-128 was expected 
> about  5 time faster.
> 
> So, why?
> 
> Pgcrypto use OpenSSL as backend, so, does it explicit force software aes 
> calculation instead of AES-NI cpu ones?

I suspect it is actually using AES hardware support, but all the other 
overhead of pgcrypto makes the difference not noticeable.







^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Is OpenSSL AES-NI not available in pgcrypto?
@ 2023-01-03 16:07  [email protected] <[email protected]>
  parent: Peter Eisentraut <[email protected]>
  0 siblings, 0 replies; 5+ messages in thread

From: [email protected] @ 2023-01-03 16:07 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; [email protected]

Hi,

I see, I was hoping that wasn't the case.

Thanks a lot for your support.

My best regards,

Agharta


Il 03/01/23 16:54, Peter Eisentraut ha scritto:
> On 02.01.23 17:57, [email protected] wrote:
>> select pgp_sym_encrypt(data::text, 'pwd') --default to aes128
>> from generate_series('2022-01-01'::timestamp, 
>> '2022-12-31'::timestamp, '1 hour'::interval) data
>>
>> vs
>>
>> select pgp_sym_encrypt(data::text, 'pwd','cipher-algo=bf') -- blowfish
>> from generate_series('2022-01-01'::timestamp, 
>> '2022-12-31'::timestamp, '1 hour'::interval) data
>>
>> In my test both queries execution is similar....aes-128 was expected 
>> about  5 time faster.
>>
>> So, why?
>>
>> Pgcrypto use OpenSSL as backend, so, does it explicit force software 
>> aes calculation instead of AES-NI cpu ones?
>
> I suspect it is actually using AES hardware support, but all the other 
> overhead of pgcrypto makes the difference not noticeable.
>






^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Is OpenSSL AES-NI not available in pgcrypto?
@ 2023-01-07 02:13  Bruce Momjian <[email protected]>
  parent: [email protected] <[email protected]>
  1 sibling, 0 replies; 5+ messages in thread

From: Bruce Momjian @ 2023-01-07 02:13 UTC (permalink / raw)
  To: [email protected] <[email protected]>; +Cc: [email protected]

On Mon, Jan  2, 2023 at 05:57:38PM +0100, [email protected] wrote:
> So, a test with pgcrypto:
> 
> select pgp_sym_encrypt(data::text, 'pwd') --default to aes128
> from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, '1
> hour'::interval) data
> 
> vs
> 
> select pgp_sym_encrypt(data::text, 'pwd','cipher-algo=bf') -- blowfish
> from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, '1
> hour'::interval) data

To see the difference, I think you need to construct a single large
query that calls many pgcrypto functions, with a small return result, so
the network, parsing, and optimizer overhead are minimal compared to the
OpenSSL overhread.

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

Embrace your flaws.  They make you human, rather than perfect,
which you will never be.






^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2023-01-07 02:13 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 06:43 [PATCH 03/10] Make sure published XIDs are persistent Kyotaro Horiguchi <[email protected]>
2023-01-02 16:57 Is OpenSSL AES-NI not available in pgcrypto? [email protected] <[email protected]>
2023-01-03 15:54 ` Re: Is OpenSSL AES-NI not available in pgcrypto? Peter Eisentraut <[email protected]>
2023-01-03 16:07   ` Re: Is OpenSSL AES-NI not available in pgcrypto? [email protected] <[email protected]>
2023-01-07 02:13 ` Re: Is OpenSSL AES-NI not available in pgcrypto? Bruce Momjian <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox