Re: unlogged tables vs. GIST - Mailing list pgsql-hackers

From Jeevan Chalke
Subject Re: unlogged tables vs. GIST
Date
Msg-id CAM2+6=UTy=Ly6VfhP_ni8BJtuyeFEtM_H6Ms=-OUCLVcBmA+sw@mail.gmail.com
Whole thread Raw
In response to Re: unlogged tables vs. GIST  (Heikki Linnakangas <hlinnakangas@vmware.com>)
Responses Re: unlogged tables vs. GIST
List pgsql-hackers
Hi Heikki,


On Mon, Jan 28, 2013 at 2:34 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote:
On 23.01.2013 17:30, Robert Haas wrote:
On Wed, Jan 23, 2013 at 4:04 AM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com>  wrote:
I guess my earlier patch, which was directly incrementing

ControlFile->unloggedLSN counter was the concern as it will take
ControlFileLock several times.

In this version of patch I did what Robert has suggested. At start of the
postmaster, copying unloggedLSn value to XLogCtl, a shared memory struct.
And
in all access to unloggedLSN, using this shared variable using a SpinLock.
And since we want to keep this counter persistent across clean shutdown,
storing it in ControlFile before updating it.

With this approach, we are updating ControlFile only when we shutdown the
server, rest of the time we are having a shared memory counter. That means
we
are not touching pg_control every other millisecond or so. Also since we are
not caring about crashes, XLogging this counter like OID counter is not
required as such.

On a quick read-through this looks reasonable to me, but others may
have different opinions, and I haven't reviewed in detail.
> ...
[a couple of good points]

In addition to those things Robert pointed out:

/*
+ * Temporary GiST indexes are not WAL-logged, but we need LSNs to detect
+ * concurrent page splits anyway. GetXLogRecPtrForUnloggedRel() provides a fake
+ * sequence of LSNs for that purpose. Each call generates an LSN that is
+ * greater than any previous value returned by this function in the same
+ * session using static counter
+ * Similarily unlogged GiST indexes are also not WAL-logged. But we need a
+ * persistent counter across clean shutdown. Use counter from ControlFile which
+ * is copied in XLogCtl.unloggedLSN to accomplish that
+ * If relation is UNLOGGED, return persistent counter from XLogCtl else return
+ * session wide temporary counter
+ */
+XLogRecPtr
+GetXLogRecPtrForUnloggedRel(Relation rel)

>From a modularity point of view, it's not good that you xlog.c needs to know about Relation struct. Perhaps move the logic to check the relation is unlogged or not to a function in gistutil.c, and only have a small GetUnloggedLSN() function in xlog.c

I kept a function as is, but instead sending Relation as a parameter, it now takes boolean, isUnlogged. Added a MACRO for that.
 

I'd suggest adding a separate spinlock to protect unloggedLSN. I'm not sure if there's much contention on XLogCtl->info_lck today, but nevertheless it'd be better to keep this separate, also from a modularity point of view.

Hmm. OK.
Added ulsn_lck for this.
 

@@ -7034,6 +7078,8 @@ CreateCheckPoint(int flags)
                LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
                ControlFile->state = DB_SHUTDOWNING;
                ControlFile->time = (pg_time_t) time(NULL);
+               /* Store unloggedLSN value as we want it persistent across shutdown */
+               ControlFile->unloggedLSN = XLogCtl->unloggedLSN;
                UpdateControlFile();
                LWLockRelease(ControlFileLock);
        }

This needs to acquire the spinlock to read unloggedLSN.

OK. Done.
 

Do we need to do anything to unloggedLSN in pg_resetxlog?

I guess NO.

Also, added Robert's comment in bufmgr.c
I am still unsure about the spinlock around buf flags as we are just examining it.

Please let me know if I missed any.

Thanks
 

- Heikki



--
Jeevan B Chalke
Senior Software Engineer, R&D
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Phone: +91 20 30589500

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the individual or entity to whom it is addressed. This message contains information from EnterpriseDB Corporation that may be privileged, confidential, or exempt from disclosure under applicable law. If you are not the intended recipient or authorized to receive this for the intended recipient, any use, dissemination, distribution, retention, archiving, or copying of this communication is strictly prohibited. If you have received this e-mail in error, please notify the sender immediately by reply e-mail and delete this message.
Attachment

pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Next
From: Bruce Momjian
Date:
Subject: Re: pg_ctl idempotent option