On 7/13/20 11:46 AM, movead...@highgo.ca wrote:
I continue to see your patch. Some code improvements see at the attachment.
Questions:
* csnSnapshotActive is the only member of the CSNshapshotShared struct.
* The WriteAssignCSNXlogRec() routine. I din't understand why you add 20
nanosec to current CSN and write this into the WAL. For simplify our
communication, I rewrote this routine in accordance with my opinion (see
patch in attachment).
At general, maybe we will add your WAL writing CSN machinery + TAP tests
to the patch from the thread [1] and work on it together?
[1]
https://www.postgresql.org/message-id/flat/07b2c899-4ed0-4c87-1327-23c750311248%40postgrespro.ru
--
regards,
Andrey Lepikhov
Postgres Professional
>From 9a1595507c83b5fde61a6a3cc30f6df9df410e76 Mon Sep 17 00:00:00 2001
From: Andrey Lepikhov <a.lepik...@postgrespro.ru>
Date: Wed, 15 Jul 2020 11:55:00 +0500
Subject: [PATCH] 1
---
src/backend/access/transam/csn_log.c | 35 ++++++++++++--------------
src/include/access/csn_log.h | 8 +++---
src/test/regress/expected/sysviews.out | 3 ++-
3 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/src/backend/access/transam/csn_log.c b/src/backend/access/transam/csn_log.c
index 319e89c805..53d3877851 100644
--- a/src/backend/access/transam/csn_log.c
+++ b/src/backend/access/transam/csn_log.c
@@ -150,8 +150,8 @@ CSNLogSetCSN(TransactionId xid, int nsubxids,
*/
static void
CSNLogSetPageStatus(TransactionId xid, int nsubxids,
- TransactionId *subxids,
- XidCSN csn, int pageno)
+ TransactionId *subxids,
+ XidCSN csn, int pageno)
{
int slotno;
int i;
@@ -187,8 +187,8 @@ CSNLogSetCSNInSlot(TransactionId xid, XidCSN csn, int slotno)
Assert(LWLockHeldByMe(CSNLogControlLock));
- ptr = (XidCSN *) (CsnlogCtl->shared->page_buffer[slotno] + entryno * sizeof(XLogRecPtr));
-
+ ptr = (XidCSN *) (CsnlogCtl->shared->page_buffer[slotno] +
+ entryno * sizeof(XidCSN));
*ptr = csn;
}
@@ -205,17 +205,16 @@ CSNLogGetCSNByXid(TransactionId xid)
int pageno = TransactionIdToPage(xid);
int entryno = TransactionIdToPgIndex(xid);
int slotno;
- XidCSN *ptr;
- XidCSN xid_csn;
+ XidCSN csn;
/* lock is acquired by SimpleLruReadPage_ReadOnly */
slotno = SimpleLruReadPage_ReadOnly(CsnlogCtl, pageno, xid);
- ptr = (XidCSN *) (CsnlogCtl->shared->page_buffer[slotno] + entryno * sizeof(XLogRecPtr));
- xid_csn = *ptr;
+ csn = *(XidCSN *) (CsnlogCtl->shared->page_buffer[slotno] +
+ entryno * sizeof(XidCSN));
LWLockRelease(CSNLogControlLock);
- return xid_csn;
+ return csn;
}
/*
@@ -501,15 +500,15 @@ WriteAssignCSNXlogRec(XidCSN xidcsn)
{
XidCSN log_csn = 0;
- if(xidcsn > get_last_log_wal_csn())
- {
- log_csn = CSNAddByNanosec(xidcsn, 20);
- set_last_log_wal_csn(log_csn);
- }
- else
- {
+ if(xidcsn <= get_last_log_wal_csn())
+ /*
+ * WAL-write related code. If concurrent backend already wrote into WAL
+ * its CSN with bigger value it isn't needed to write this value.
+ */
return;
- }
+
+ log_csn = CSNAddByNanosec(xidcsn, 0);
+ set_last_log_wal_csn(log_csn);
XLogBeginInsert();
XLogRegisterData((char *) (&log_csn), sizeof(XidCSN));
@@ -571,7 +570,6 @@ csnlog_redo(XLogReaderState *record)
LWLockAcquire(CSNLogControlLock, LW_EXCLUSIVE);
set_last_max_csn(csn);
LWLockRelease(CSNLogControlLock);
-
}
else if (info == XLOG_CSN_SETXIDCSN)
{
@@ -589,7 +587,6 @@ csnlog_redo(XLogReaderState *record)
SimpleLruWritePage(CsnlogCtl, slotno);
LWLockRelease(CSNLogControlLock);
Assert(!CsnlogCtl->shared->page_dirty[slotno]);
-
}
else if (info == XLOG_CSN_TRUNCATE)
{
diff --git a/src/include/access/csn_log.h b/src/include/access/csn_log.h
index 5838028a30..c23a71446a 100644
--- a/src/include/access/csn_log.h
+++ b/src/include/access/csn_log.h
@@ -15,10 +15,10 @@
#include "utils/snapshot.h"
/* XLOG stuff */
-#define XLOG_CSN_ASSIGNMENT 0x00
-#define XLOG_CSN_SETXIDCSN 0x10
-#define XLOG_CSN_ZEROPAGE 0x20
-#define XLOG_CSN_TRUNCATE 0x30
+#define XLOG_CSN_ASSIGNMENT 0x00
+#define XLOG_CSN_SETXIDCSN 0x10
+#define XLOG_CSN_ZEROPAGE 0x20
+#define XLOG_CSN_TRUNCATE 0x30
typedef struct xl_xidcsn_set
{
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 06c4c3e476..cc169a1999 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -73,6 +73,7 @@ select name, setting from pg_settings where name like 'enable%';
name | setting
--------------------------------+---------
enable_bitmapscan | on
+ enable_csn_snapshot | off
enable_gathermerge | on
enable_hashagg | on
enable_hashjoin | on
@@ -90,7 +91,7 @@ select name, setting from pg_settings where name like 'enable%';
enable_seqscan | on
enable_sort | on
enable_tidscan | on
-(18 rows)
+(19 rows)
-- Test that the pg_timezone_names and pg_timezone_abbrevs views are
-- more-or-less working. We can't test their contents in any great detail
--
2.25.1
