unlogged tables vs. GIST

On 14.12.2010 23:06, Robert Haas wrote:

On Sat, Nov 13, 2010 at 9:09 PM, Robert Haas<robertmhaas@gmail.com> wrote:

The fact that it's easy doesn't make it workable. I would point out for
starters that AMs might (do) put WAL locations and/or XIDs into indexes.
Occasionally copying very old LSNs or XIDs back into active files seems
pretty dangerous.

I haven't examined the GIST, GIN, or hash index code in detail so I am
not sure whether there are any hazards there; the btree case does not
seem to have any issues of this type. Certainly, if an index AM puts
an XID into an empty index, that's gonna break. I would consider that
a pretty odd thing to do, though. An LSN seems less problematic since
the LSN space does not wrap; it should just look like an index that
was created a long time ago and never updated (which, in effect, it
is).

I'm still not convinced there's any hazard of this type, but there is,
apparently, a problem with failing to emit XLOG records for GIST
indexes, because they apparently use LSNs to detect concurrent page
splits (see Heikki's commit on November 16th, aka
2edc5cd493ce3d7834026970e9d3cd00e203f51a) and the hack he inserted to
work around that problem for temporary tables isn't going to work for
unlogged tables. I suppose we could disallow unlogged GIST indexes.
Or we could allow them but still XLOG some operations anyway to make
sure that the LSN advances at the appropriate time. That seems pretty
ugly, though. Any other ideas?

Hmm, the first idea that comes to mind is to use a counter like the
GetXLogRecPtrForTemp() counter I used for temp tables, but global, in
shared memory. However, that's a bit problematic because if we store a
value from that counter to LSN, it's possible that the counter overtakes
the XLOG insert location, and you start to get xlog flush errors. We
could avoid that if we added a new field to the GiST page header, and
used that to store the value in the parent page instead of the LSN.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, Dec 14, 2010 at 4:24 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Hmm, the first idea that comes to mind is to use a counter like the
GetXLogRecPtrForTemp() counter I used for temp tables, but global, in shared
memory. However, that's a bit problematic because if we store a value from
that counter to LSN, it's possible that the counter overtakes the XLOG
insert location, and you start to get xlog flush errors. We could avoid that
if we added a new field to the GiST page header, and used that to store the
value in the parent page instead of the LSN.

That doesn't seem ideal, either, because now you're eating up some
number of bytes per page in every GIST index just on the off chance
that one of them is unlogged. Unless there's a way to do it only for
unlogged GIST indexes, but it seems like that could be messy.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 14, 2010 at 4:24 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Hmm, the first idea that comes to mind is to use a counter like the
GetXLogRecPtrForTemp() counter I used for temp tables, but global, in shared
memory. However, that's a bit problematic because if we store a value from
that counter to LSN, it's possible that the counter overtakes the XLOG
insert location, and you start to get xlog flush errors. We could avoid that
if we added a new field to the GiST page header, and used that to store the
value in the parent page instead of the LSN.

That doesn't seem ideal, either, because now you're eating up some
number of bytes per page in every GIST index just on the off chance
that one of them is unlogged.

On-disk compatibility seems problematic here as well.

regards, tom lane

On Tue, Dec 14, 2010 at 4:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 14, 2010 at 4:24 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Hmm, the first idea that comes to mind is to use a counter like the
GetXLogRecPtrForTemp() counter I used for temp tables, but global, in shared
memory. However, that's a bit problematic because if we store a value from
that counter to LSN, it's possible that the counter overtakes the XLOG
insert location, and you start to get xlog flush errors. We could avoid that
if we added a new field to the GiST page header, and used that to store the
value in the parent page instead of the LSN.

That doesn't seem ideal, either, because now you're eating up some
number of bytes per page in every GIST index just on the off chance
that one of them is unlogged.

On-disk compatibility seems problematic here as well.

Good point.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Dec 14, 2010 at 5:14 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Dec 14, 2010 at 4:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Dec 14, 2010 at 4:24 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Hmm, the first idea that comes to mind is to use a counter like the
GetXLogRecPtrForTemp() counter I used for temp tables, but global, in shared
memory. However, that's a bit problematic because if we store a value from
that counter to LSN, it's possible that the counter overtakes the XLOG
insert location, and you start to get xlog flush errors. We could avoid that
if we added a new field to the GiST page header, and used that to store the
value in the parent page instead of the LSN.

That doesn't seem ideal, either, because now you're eating up some
number of bytes per page in every GIST index just on the off chance
that one of them is unlogged.

On-disk compatibility seems problematic here as well.

Good point.

Given the foregoing discussion, I see only two possible paths forward here.

1. Just decide that that unlogged tables can't have GIST indexes, at
least until someone figures out a way to make it work. That's sort of
an annoying limitation, but I think we could live with it.

2. Write WAL records even though the GIST index is supposedly
unlogged. We could either (1) write the usual XLOG_GIST_* records,
and arrange to ignore them on replay when the relation in question is
unlogged, or (2) write an XLOG_NOOP record to advance the current LSN.
The latter sounds safer to me, but it will mean that the XLOG code
for GIST needs three separate cases (temp, perm, unlogged). Either
way we give up a significant chunk of the benefit of making the
relation unlogged in the first place.

Thoughts?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Given the foregoing discussion, I see only two possible paths forward here.

1. Just decide that that unlogged tables can't have GIST indexes, at
least until someone figures out a way to make it work. That's sort of
an annoying limitation, but I think we could live with it.

+1

In the small subset of situations that need unlogged tables, I would
think the subset of those that need gist is exceedingly small.

Unless someone can come up with a use case that needs both unlogged and
gist, I'd vote not to spend time on it. (And, if ever someone does come
along with a really good use, then time can be put toward it).

-Andy

Robert Haas <robertmhaas@gmail.com> writes:

Given the foregoing discussion, I see only two possible paths forward here.

1. Just decide that that unlogged tables can't have GIST indexes, at
least until someone figures out a way to make it work. That's sort of
an annoying limitation, but I think we could live with it.

2. Write WAL records even though the GIST index is supposedly
unlogged. We could either (1) write the usual XLOG_GIST_* records,
and arrange to ignore them on replay when the relation in question is
unlogged, or (2) write an XLOG_NOOP record to advance the current LSN.
The latter sounds safer to me, but it will mean that the XLOG code
for GIST needs three separate cases (temp, perm, unlogged). Either
way we give up a significant chunk of the benefit of making the
relation unlogged in the first place.

Thoughts?

IIUC, the problem is that the bufmgr might think that a GIST NSN is an
LSN that should affect when to force out a dirty buffer? What if we
taught it the difference? We could for example dedicate a pd_flags
bit to marking pages whose pd_lsn isn't actually an LSN.

This solution would probably imply that all pages in the shared buffer
pool have to have a standard PageHeaderData header, not just an LSN at
the front as is assumed now. But that doesn't seem like a bad thing to
me, unless maybe we were dumb enough to not use a standard page header
in some of the secondary forks.

regards, tom lane

On 17.12.2010 21:07, Tom Lane wrote:

IIUC, the problem is that the bufmgr might think that a GIST NSN is an
LSN that should affect when to force out a dirty buffer? What if we
taught it the difference? We could for example dedicate a pd_flags
bit to marking pages whose pd_lsn isn't actually an LSN.

This solution would probably imply that all pages in the shared buffer
pool have to have a standard PageHeaderData header, not just an LSN at
the front as is assumed now. But that doesn't seem like a bad thing to
me, unless maybe we were dumb enough to not use a standard page header
in some of the secondary forks.

I'm not very fond of expanding buffer manager's knowledge of the page
layout. How about a new flag in the buffer desc, BM_UNLOGGED? There was
some talk about skipping flushing of unlogged tables at checkpoints, I
think we'd need BM_UNLOGGED for that anyway. Or I guess we could hang
that behavior on the pd_flags bit too, but it doesn't seem like the
right place for that information.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

On 17.12.2010 21:07, Tom Lane wrote:

IIUC, the problem is that the bufmgr might think that a GIST NSN is an
LSN that should affect when to force out a dirty buffer? What if we
taught it the difference? We could for example dedicate a pd_flags
bit to marking pages whose pd_lsn isn't actually an LSN.

I'm not very fond of expanding buffer manager's knowledge of the page
layout. How about a new flag in the buffer desc, BM_UNLOGGED?

That could work too, if you can explain how the flag comes to be set
without a bunch of ugliness all over the system. I don't want callers
of ReadBuffer to have to supply the bit for example.

regards, tom lane

On Fri, Dec 17, 2010 at 2:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:
IIUC, the problem is that the bufmgr might think that a GIST NSN is an
LSN that should affect when to force out a dirty buffer?  What if we
taught it the difference?  We could for example dedicate a pd_flags
bit to marking pages whose pd_lsn isn't actually an LSN.

This solution would probably imply that all pages in the shared buffer
pool have to have a standard PageHeaderData header, not just an LSN at
the front as is assumed now.  But that doesn't seem like a bad thing to
me, unless maybe we were dumb enough to not use a standard page header
in some of the secondary forks.

Ah, interesting idea. visibilitymap.c has this:

#define MAPSIZE (BLCKSZ - MAXALIGN(SizeOfPageHeaderData))

and fsm_internals.h has this:

#define NodesPerPage (BLCKSZ - MAXALIGN(SizeOfPageHeaderData) - \
offsetof(FSMPageData, fp_nodes))

...which seems to imply, at least on a quick look, that we might be
OK, as far as the extra forks go.

I have a vague recollection that the documentation says somewhere that
a full page header on every page isn't required. And I'm a little
worried about my ability to track down every place in the system that
might get broken by a change in this area. What do you think about
committing the unlogged tables patch initially without support for
GIST indexes, and working on fixing this as a separate commit?

Another possibly-useful thing about mandating a full page header for
every page is that it might give us a way of avoiding unnecessary full
page writes. As I wrote previously:

However, I think we can avoid this too, by
allocating an additional bit in pd_flags, PD_FPI. Instead of emitting
an FPI when the old LSN precedes the redo pointer, we'll emit an FPI
when the FPI bit is set (in which case we'll also clear the bit) OR
when the old LSN precedes the redo pointer. Upon emitting a WAL
record that is torn-page safe (such as a freeze or all-visible
record), we'll pass a flag to XLogInsert that arranges to suppress
FPIs, bump the LSN, and set PD_FPI. That way, if the page is touched
again before the next checkpoint by an operation that does NOT
suppress FPI, one will be emitted then.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

Another possibly-useful thing about mandating a full page header for
every page is that it might give us a way of avoiding unnecessary full
page writes. As I wrote previously:

Could we do that via a bufmgr status bit, instead? Heikki's idea has
the merit that it actually reduces bufmgr's knowledge of page headers,
rather than increasing it (since a buffer marked UNLOGGED would need
no assumptions at all about its content).

regards, tom lane

On Fri, Dec 17, 2010 at 2:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

On 17.12.2010 21:07, Tom Lane wrote:

IIUC, the problem is that the bufmgr might think that a GIST NSN is an
LSN that should affect when to force out a dirty buffer?  What if we
taught it the difference?  We could for example dedicate a pd_flags
bit to marking pages whose pd_lsn isn't actually an LSN.

I'm not very fond of expanding buffer manager's knowledge of the page
layout. How about a new flag in the buffer desc, BM_UNLOGGED?

That could work too, if you can explain how the flag comes to be set
without a bunch of ugliness all over the system.  I don't want callers
of ReadBuffer to have to supply the bit for example.

That's easy enough to solve - ReadBuffer() takes a Relation as an
argument, so you can easily check RelationNeedsWAL(reln).

I guess the question is whether it's right to conflate "table is
unlogged" with "LSN is fake". It's not immediately obvious to me that
those concepts are isomorphic, although though the reverse isn't
obvious to me either.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Dec 17, 2010 at 2:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Another possibly-useful thing about mandating a full page header for
every page is that it might give us a way of avoiding unnecessary full
page writes.  As I wrote previously:

Could we do that via a bufmgr status bit, instead?  Heikki's idea has
the merit that it actually reduces bufmgr's knowledge of page headers,
rather than increasing it (since a buffer marked UNLOGGED would need
no assumptions at all about its content).

That was my first thought, but it doesn't work. The buffer could be
evicted from shared_buffers and read back in. If a checkpoint
intervenes meanwhile, we're OK, but otherwise you fail to emit an
otherwise-needed FPI.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 17.12.2010 21:32, Robert Haas wrote:

I guess the question is whether it's right to conflate "table is
unlogged" with "LSN is fake". It's not immediately obvious to me that
those concepts are isomorphic, although though the reverse isn't
obvious to me either.

The buffer manager only needs to know if it has to flush the WAL before
writing the page to disk. The flag just means that the buffer manager
never needs to do that for this buffer. You're still free to store a
real LSN there if you want to, it just won't cause any WAL flushes.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

On 17.12.2010 21:32, Robert Haas wrote:

I guess the question is whether it's right to conflate "table is
unlogged" with "LSN is fake". It's not immediately obvious to me that
those concepts are isomorphic, although though the reverse isn't
obvious to me either.

The buffer manager only needs to know if it has to flush the WAL before
writing the page to disk. The flag just means that the buffer manager
never needs to do that for this buffer. You're still free to store a
real LSN there if you want to, it just won't cause any WAL flushes.

Yeah. I think that BM_UNLOGGED might be a poor choice for the flag name,
just because it overstates what the bufmgr needs to assume. It might be
better to reverse the flag sense, and have a new flag that *is* set if
the page contains an LSN that we have to check against WAL.

regards, tom lane

On Fri, Dec 17, 2010 at 3:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

On 17.12.2010 21:32, Robert Haas wrote:

I guess the question is whether it's right to conflate "table is
unlogged" with "LSN is fake".  It's not immediately obvious to me that
those concepts are isomorphic, although though the reverse isn't
obvious to me either.

The buffer manager only needs to know if it has to flush the WAL before
writing the page to disk. The flag just means that the buffer manager
never needs to do that for this buffer. You're still free to store a
real LSN there if you want to, it just won't cause any WAL flushes.

Yeah.  I think that BM_UNLOGGED might be a poor choice for the flag name,
just because it overstates what the bufmgr needs to assume.  It might be
better to reverse the flag sense, and have a new flag that *is* set if
the page contains an LSN that we have to check against WAL.

I was actually thinking of adding BM_UNLOGGED even before this
discussion, because that would allow unlogged buffers to be excluded
from non-shutdown checkpoints. We could add two flags with different
semantics that take on, under present rules, the same value, but I'd
be disinclined to burn the extra bit without a concrete need.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Dec 17, 2010 at 3:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah. �I think that BM_UNLOGGED might be a poor choice for the flag name,
just because it overstates what the bufmgr needs to assume.

I was actually thinking of adding BM_UNLOGGED even before this
discussion, because that would allow unlogged buffers to be excluded
from non-shutdown checkpoints. We could add two flags with different
semantics that take on, under present rules, the same value, but I'd
be disinclined to burn the extra bit without a concrete need.

bufmgr is currently using eight bits out of a 16-bit flag field, and
IIRC at least five of those have been there since the beginning. So our
accretion rate is something like one bit every four years. I think not
being willing to use two bits to describe two unrelated behaviors is
penny-wise and pound-foolish --- bufmgr is already complicated enough,
let's not add useless barriers to readability.

regards, tom lane

On Fri, Dec 17, 2010 at 3:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Dec 17, 2010 at 3:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah.  I think that BM_UNLOGGED might be a poor choice for the flag name,
just because it overstates what the bufmgr needs to assume.

I was actually thinking of adding BM_UNLOGGED even before this
discussion, because that would allow unlogged buffers to be excluded
from non-shutdown checkpoints.  We could add two flags with different
semantics that take on, under present rules, the same value, but I'd
be disinclined to burn the extra bit without a concrete need.

bufmgr is currently using eight bits out of a 16-bit flag field, and
IIRC at least five of those have been there since the beginning.  So our
accretion rate is something like one bit every four years.  I think not
being willing to use two bits to describe two unrelated behaviors is
penny-wise and pound-foolish --- bufmgr is already complicated enough,
let's not add useless barriers to readability.

Allright, what do you want to call the other bit, then? BM_SKIP_XLOG_FLUSH?

I have a feeling we may also be creating BM_UNTIDY rather soon, per
previous discussion of hint bit I/O.

Since these bits will only be set/cleared when the buffer mapping is
changed, can we examine this bit without taking the spinlock? If not,
we're going to have to stick an extra spinlock acquire/release into
FlushBuffer(), which sounds rather unappealing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

Allright, what do you want to call the other bit, then? BM_SKIP_XLOG_FLUSH?

Like I said, I'd be tempted to invert the sense, so that the flag is set
for normal relations. Then it becomes something like BM_FLUSH_XLOG.

regards, tom lane