On Tue, Nov 14, 2017 at 10:01 AM, Peter Geoghegan <p...@bowt.ie> wrote:
> On Tue, Nov 14, 2017 at 1:41 AM, Rushabh Lathia
> <rushabh.lat...@gmail.com> wrote:
>> Thanks Peter and Thomas for the review comments.
>
> No problem. More feedback:
I see that
model has always forced users to be far too conservative.
Workloads are very complicated, and always having users target the
worst case leaves a lot to be desired.
--
Peter Geoghegan
tables,
> would it be ok to skip the lazy_cleanup_index call? As we are sure we did
> not touch the heap or the index, I'd say a cleanup may not be necessary.
There is a patch in the ongoing CF to do this:
https://commitfest.postgresql.org/15/952/
It's a lot harder to do this correctly than it first appears.
--
Peter Geoghegan
laiming
them for the FSM the next time around (maybe that's the VACUUM that
enables advancing the epoch).
We cannot break that assumption, and no easy fixes suggest themselves.
--
Peter Geoghegan
and B-Tree indexes are read sequentially during VACUUM.
Often, autovacuum runs at a much slower rate than is actually
possible, which isn't necessarily the right trade-off.
--
Peter Geoghegan
nition supposed to be there?
--
Peter Geoghegan
his for the
red-black tree library code, for example, and it seems like good
practice.
Would that address your concern? There would be an SQL interface, but
it would be trivial.
--
Peter Geoghegan
interface that simply invokes the test harness, with all
the heavy lifting taking place in C code.
Obviously these are two very different things. I'm quite happy to add
the test harness.
--
Peter Geoghegan
On Wed, Nov 29, 2017 at 8:34 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> The patch does not currently apply. I am noticing as well that Peter
> Geoghegan has registered himself as a reviewer a couple of hours back,
> so moved to next CF with waiting on author as
t in
> addition to removing IndexTupleDSize as proposed, we also remove that
> cast. It seems that the only place which relies on that cast
> currently is btree_xlog_split.
>
> Therefore I propose the attached patch instead.
+1 to both points. I specifically recall being annoyed by both issu
x pages that contain a cached
XID -- the one we compare to RecentGlobalXmin as part of the recycling
interlock. (I suggested this to Sawada-san at one point, in the
context of avoiding vacuuming indexes on large append-mostly tables.)
In any case, we'd hardly go to all that effort to save just 4 bytes in
the page header.
--
Peter Geoghegan
locking at some central
choke point), then you risk concurrent recycling races. I am wearing
my nbtree hat in assessing the GIN code, which admittedly might not
always be the best way of looking at it. I'm explaining my perspective
because I think it might be useful for you to understand my thought
process. I hope I don't come off as peevish.
--
Peter Geoghegan
but 9.3+ works
too.
Note that somebody asked about running it on Windows recently, and on
one other occasion in the past. It does come up.
--
Peter Geoghegan
$0 instead of a hardcoded name in the header actually buys
> us anything afaict.
+1.
I think that reproducible builds are a worthwhile goal, and I welcome
Christoph's continued work on them.
--
Peter Geoghegan
amcheck on the Heroku fleet, back when I worked there, there were all
kinds of non-interesting errors that could occur that needed to be
filtered out. I want to try to make that process somewhat less painful.
--
Peter Geoghegan
t code
> has changed a bit between branches. Happy to do so on master.
The elog(), which was itself upgraded from a simple Assert by commit
d70cf811, appears in exactly the same form in 9.3+. Things did change
there, but they were kept in sync.
--
Peter Geoghegan
On Fri, Dec 15, 2017 at 11:03 AM, Peter Geoghegan <p...@bowt.ie> wrote:
> The elog(), which was itself upgraded from a simple Assert by commit
> d70cf811, appears in exactly the same form in 9.3+. Things did change
> there, but they were kept in sync.
BTW, if you're going to do it,
jwwgucxeh3d1...@mail.gmail.com
--
Peter Geoghegan
From 0a37ceea53736e39d0f1af72ec9bc07d98833370 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <p...@bowt.ie>
Date: Fri, 15 Dec 2017 14:05:16 -0800
Subject: [PATCH] Promote "HOT parent tuple" elog to an ereport.
Commit
ates is a good idea, sometimes rounding can cause confusion.
The bloom filter doesn't track the number of elements itself. Callers
that care can do it themselves. bloom_prop_bits_set() isn't very
important, even compared to other types of instrumentation. It's
moderately useful as a generic indicator of whether or not the Bloom
filter was totally overwhelmed. That's about it.
--
Peter Geoghegan
Yes, you have that right. Per dumptuples(), even the zero tuple run edge
case will still write a run marker, and will therefore still consume a
tape. We must have at least two initial runs to merge. (though dummy runs
for non final merges are a slightly different matter.)
--
Peter Geoghegan
(Sent
On Wed, Oct 18, 2017 at 12:45 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> Bringing it back to the concrete freeze-the-dead issue, and the
> question of an XID-cutoff for safely interrogating CLOG: I guess it
> will be possible to assess a HOT chain as a whole. We can probably d
and since a new GUC seems like it would noticeably improve
matters, I am beginning to take the idea of writing a hash_mem patch
for v11 seriously.
--
Peter Geoghegan
lower
than hash aggregates, while still using approximately the same amount
of memory as before. This creates significantly more pressure quite
suddenly, because the group aggregates are quite a bit slower, and it
takes that much longer for the memory to be released.
I'm mostly concerned about avoiding instability like this. Users
greatly value predictable performance.
--
Peter Geoghegan
ably has fundamental design
problems (it's not *just* buggy).
[1]
https://postgr.es/m/CAH2-WznBt2+7qc65btjxNNwa9BW+jKEBgVjb=f+26iuqhmy...@mail.gmail.com
--
Peter Geoghegan
an important
patch, that has fallen through the cracks far too many times.
--
Peter Geoghegan
t; elog errors, as a defensive measure, in both
scanPendingInsert() and ginInsertCleanup(). The 2013 bug fix actually
did this for the posting tree, but didn't touch similar pending list
code.
--
Peter Geoghegan
ses, etc. (I bet the v10 enhancements
disproportionately improved CLUSTER performance.)
--
Peter Geoghegan
ents, I think. At least having something that you can
Google can make all the difference.
--
Peter Geoghegan
exPage() in GIN.
--
Peter Geoghegan
ve
far more questions than answers right now, though.
--
Peter Geoghegan
On Thu, Nov 16, 2017 at 12:49 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> But there is only ever one page locked, the meta-page. And, it's
> always an ExclusiveLock. I don't see much use for deadlock avoidance.
I meant deadlock detection.
--
Peter Geoghegan
r your input.
Thanks for listening. I regret that it became heated, but I'm glad
that we now understand each other's perspective. I'm also glad that
you're pushing ahead with MERGE as a project, because MERGE is
certainly a compelling feature.
--
Peter Geoghegan
eferable to what we have now.
--
Peter Geoghegan
(We
* don't need to remember the memory context we're using explicitly,
* because after creation we only repalloc our arrays larger.)
*/
ResourceOwner resowner;
Maybe we need to remember the original caller's memory context, too?
Either that, or the contract/comments for memory contexts need to be
revised.
--
Peter Geoghegan
rk_mem each worker tuplesort
gets. Our planner code needs to take that into account, now that the
nbtsort.c parallel_leader_participation behavior isn't just some
obscure debug option. IOW, the planner code needs to be consistent
with the nbtsort.c execution code.
--
Peter Geoghegan
On Tue, Dec 5, 2017 at 9:09 AM, Peter Geoghegan <p...@bowt.ie> wrote:
>> Committed with these additions. Please check that I haven't messed anything
>> up.
>
> Thanks, but you modified RecentGlobalDataXmin, not RecentGlobalXmin.
> Can you fix it, please?
I was
come up.
>
> Committed with these additions. Please check that I haven't messed anything
> up.
Thanks, but you modified RecentGlobalDataXmin, not RecentGlobalXmin.
Can you fix it, please?
--
Peter Geoghegan
.org/message-id/CANP8%2BjLf6ozqc2ie30%2B--h6oxFUL48jmjQXPS5dpiTFtvtYPYQ%40mail.gmail.com
--
Peter Geoghegan
On Tue, Nov 28, 2017 at 9:54 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> On Tue, Nov 28, 2017 at 9:50 PM, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
>>> Would that address your concern? There would be an SQL interface, but
>>> it would be trivial.
&
with the result of
compute_parallel_worker(), to consider whether or not a
leader-as-worker state should be used, despite the fact that no
existing compute_parallel_worker() caller does anything like this.
--
Peter Geoghegan
On Fri, May 4, 2018 at 2:39 AM, Teodor Sigaev <teo...@sigaev.ru> wrote:
> Thank you, pushed.
Thanks.
--
Peter Geoghegan
onally,
> or only if the item wasn't found in cache. There's some coherency
> differences, obviously.
+1. This would be a good option for amcheck, too.
--
Peter Geoghegan
page. Therefore I was more thinking to falling back to smgrread() or
> such.
I'm not envisaging anything specific just yet. It would be nice if
amcheck had an option that bypassed shared_buffers, because users want
that. That's all.
--
Peter Geoghegan
e to focus on making this
accessible to backup tools, that should ideally work without needing
to acquire any MVCC snapshot. Probably from a front-end utility. We'd
need to export at least some operator class functionality to make that
work, though.
--
Peter Geoghegan
On Thu, May 3, 2018 at 5:16 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> On Thu, May 3, 2018 at 12:37 PM, Robert Haas <robertmh...@gmail.com> wrote:
>> On Wed, May 2, 2018 at 3:06 PM, Vladimir Sitnikov
>> <sitnikov.vladi...@gmail.com> wrote:
>>>
little point in changing
> the error handling, still it would be good to get test coverage. That's
> not really a bug though as in all those cases we don't get errors like
> "could not open file" or such. So we could also let things as they are
> now.
Now that the relkind iss
to make it more
accessible, though it's already quite accessible.
[1] https://lwn.net/Articles/753601/
[2] https://github.com/iovisor/bcc/blob/master/tools/trace_example.txt
--
Peter Geoghegan
On Fri, May 11, 2018 at 11:46 PM, Christoph Berg <m...@debian.org> wrote:
> Re: Peter Geoghegan 2018-05-12
>
DD COLUMN sets a default value for a
column". Referencing the fact that Postgres was previously able to do
this with a NULL default doesn't seem to add anything.
--
Peter Geoghegan
On Fri, May 11, 2018 at 12:17 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> * Suggest replacement sort item be phrased as: "Remove the
> configuration parameter replacement_sort_tuples. The
> replacement selection sort algorithm is no longer used."
Also, it should be moved
ter replacement_sort_tuples. The
replacement selection sort algorithm is no longer used."
--
Peter Geoghegan
other
debating something that's inconsequential?
FWIW, I am neutral on the important question of whether or not this
patch should actually be back-patched.
--
Peter Geoghegan
to do this stuff without the user having to read a tedious explanation
of how the different parts fit together.
--
Peter Geoghegan
don't you start a thread about it? I'd actually even suggest
> backpatching it.
PostGIS has one of these too, which covers several languages,
including SQL. Perhaps it would be worth tightening up the indentation
of SQL files while we're at it.
--
Peter Geoghegan
ve.
That change was also left out. If I'm mistaken in how I interpreted
the sentence, then Andres should follow up.
Thanks
--
Peter Geoghegan
On Sat, Jun 9, 2018 at 10:40 AM, Daniel Gustafsson wrote:
> Thanks, I’m honoured to get the first commit.
Keep them coming. :-)
--
Peter Geoghegan
el as they are matched/killed. It should be safe to avoid
rechecking anything other than the heap TID values.
[1]
http://pgeoghegan.blogspot.com/2017/07/postgresql-index-bloat-microscope.html
[2]
https://www.postgresql.org/message-id/CAH2-Wzmf6intNY1ggiNzOziiO5Eq=DsXfeptODGxO=2j-i1...@mail.gmail.com
--
Peter Geoghegan
(unless perhaps you assume that that problem is
solved by something else, such as zheap). The mechanism that Andrey
describes is rather unlike VACUUM as we know it today, but that's the
whole point.
--
Peter Geoghegan
on't quite see a clear path to making this patch useful.
But, as I said, I have an open mind about what the next step should
be.
[1]
https://wiki.postgresql.org/wiki/Key_normalization#Avoiding_unnecessary_unique_index_enforcement
--
Peter Geoghegan
e we at least
know that the deleting transaction committed there. That is, they
could be recently dead or dead, and it may be worth going to the extra
trouble of checking which when we know that it's one of the two
possibilities.
--
Peter Geoghegan
On Mon, Jun 18, 2018 at 10:21 AM, Alvaro Herrera
wrote:
> One which includes at least half a working day in a different timezone.
> You asked mid-afternoon on a Friday in a timezone pretty far west.
It was 11 am PST.
I'll make a note about this. It won't happen again.
--
Peter Geoghegan
ic
collector stats masking a problem, the possibility that there are very
important queries that do use the index but are only run very
infrequently, and so on.
--
Peter Geoghegan
On Mon, Jun 18, 2018 at 2:54 PM, Peter Geoghegan wrote:
> On Sun, Jun 17, 2018 at 9:39 PM, Andrey V. Lepikhov
> wrote:
>> Patch '0001-retail-indextuple-deletion' introduce new function
>> amtargetdelete() in access method interface. Patch
>> '0002-quick-vacuum-strategy'
eature. Oracle has a similar feature.
--
Peter Geoghegan
On Mon, Jun 18, 2018 at 4:05 PM, Peter Geoghegan wrote:
> Finally, doing things this way would let you delete multiple
> duplicates in one shot, as I described in an earlier e-mail. Only a
> single descent of the tree is needed to delete quite a few index
> tuples, provided that the
On Mon, Jun 18, 2018 at 4:05 PM, Peter Geoghegan wrote:
> IOW, the approach you've taken in bttargetdelete() isn't quite correct
> because you imagine that it's okay to occasionally "lose" the index
> tuple that you originally found, and just move on. That needs to be
>
s issue is that gin indexes
> could return a correct result without no assertion failure even if it
> somewhat corrupted. So maybe having amcheck for gin indexes would
> resolve part of problems.
That seems like a really good idea. As you know, I have misgivings
about this area.
--
Peter Geoghegan
an also imagine someone
seeing something that I missed.
--
Peter Geoghegan
ue. I think we will need an option for that.
> I'm not yet sure if this option should be user-visible or
> auto-decision based on table access method used.
I agree.
--
Peter Geoghegan
z-ktrqiaa13xg1gne461yowra-s-yccqptyfrpkta...@mail.gmail.com
[3]
https://wiki.postgresql.org/wiki/Key_normalization#Suffix_truncation_of_normalized_keys
--
Peter Geoghegan
0001-Ensure-nbtree-leaf-tuple-keys-are-always-unique.patch
Description: Binary data
umns isn't appropriate. It seems sufficient to only
mention this once, in the CREATE INDEX docs.
Attached patch shows what I have in mind -- the total removal of this
top-level section.
--
Peter Geoghegan
From 293f1fc93771fa6be65867f53aa507fb30137230 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan
Dat
ection was not well considered in the first place, I
thought that the question was clear cut, and I doubted that anyone
else would follow up at all.
--
Peter Geoghegan
e -- not just the current session.
--
Peter Geoghegan
't seem like anything we can't fix with acceptable risk.
I agree with both points.
--
Peter Geoghegan
aybe that alone will be enough to make the
patch worth committing; "opportunistic microvacuuming" can come later,
if at all.
--
Peter Geoghegan
to the heap tuple via an index scan
if there are no index tuples that stick around until "recently dead"
heap tuples become "fully dead"? How can you avoid keeping around both
old and new index tuples at the same time?
--
Peter Geoghegan
On Tue, Jun 19, 2018 at 4:06 AM, Pavan Deolasee
wrote:
> I would like to resubmit the MERGE patch for PG12. The past discussions
> about the patch can be found here [1] [2].
FWIW, I'm really glad that you're going to work on this for v12.
--
Peter Geoghegan
the only one that doesn't use context diffs and also doesn't hate
them with a passion? I don't get it.
> There's already a (perhaps underdocumented) way to make regression
> diffs look the way you want in local runs.
Actually, it is documented.
--
Peter Geoghegan
On Sun, May 27, 2018 at 2:09 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> On Sun, May 27, 2018 at 12:45 PM, Jonathan S. Katz <jk...@postgresql.org>
> wrote:
>> Next, see bad.sql. 1.2MM sparsely clustered rows inserted, GiST indexes
>> builds in about 30s on my ma
Thanks!
Congratulations to the other 6 new committers.
--
Peter Geoghegan
(Sent from my phone)
s of enforcement, etc.
Naturally, the rules across disparate groups vary widely for all kinds
of reasons. Formalizing and being more transparent about how this
works seems like the opposite of paternalism to me.
--
Peter Geoghegan
l screwed in this situation. It does finish eventually, but in about
> 50x longer than GiST. I imagine the index's query performance would be
> equally awful.
Can you think of some way of side-stepping the issue? It's unfortunate
that SP-GiST is potentially so sensitive to input order.
--
Peter Geoghegan
rting about 500 million records. Periodic pstack
> runs on Linux showed that the backend was busy in btree operations. I didn't
> pursue the cause due to other businesses, but there might be something to be
> improved.
What kind of data was indexed? Was it a bigserial primary key, or
so
ral, page deletion is the most complicated part of nbtree
concurrency, by far (if we just had the basic L, the concurrency
aspects would be far easier to grasp). Doing better in
_bt_unlink_halfdead_page() seems extremely difficult, and very
unlikely to be worthwhile.
--
Peter Geoghegan
as needed.
You're probably looking at a test-case that doesn't involve a
multi-level deletion, where the leaf and target page are the same. Not
sure if you want to reproduce the multi-level deletion case
independently.
--
Peter Geoghegan
On Wed, Jun 27, 2018 at 1:11 PM, Andres Freund wrote:
> Well, I don't really want to generally do better. Just be able to check
> for interrupts ;)
That's what I meant. :-)
--
Peter Geoghegan
m
unsure of that, though.
--
Peter Geoghegan
between _bt_delitems_delete() and
_bt_delitems_vacuum(), and the considerations for hot standby? I think
that that's another TODO list item for this patch.
--
Peter Geoghegan
On Fri, Jun 22, 2018 at 12:43 PM, Peter Geoghegan wrote:
> On Fri, Jun 22, 2018 at 4:24 AM, Andrey V. Lepikhov
> wrote:
>> According to your feedback, i develop second version of the patch.
>> In this version:
>> 1. High-level functions index_beginscan(), index_rescan() n
at my patch
has are also problems for your patch. One patch cannot get committed
without the other, so they are already the same project. As a bonus,
my patch will probably improve the best case performance for your
patch, since multi-deletions will now have much better locality of
access.
--
Peter Geoghegan
t presorted
SERIAL-like inputs, especially when parallelism is used. Back in 1999,
there was no WAL-logging.
--
Peter Geoghegan
From 2a77b947e07dadef953019334062580cad560836 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan
Date: Sun, 24 Jun 2018 11:41:16 -0700
Subject: [PATCH] Remove obsolete comment
s of that technology.
I don't understand, but okay. I can provide feedback once a design for
delete marking is available.
--
Peter Geoghegan
On Thu, Jun 14, 2018 at 11:44 AM, Peter Geoghegan wrote:
> I attach an unfinished prototype of suffix truncation, that also
> sometimes *adds* a new attribute in pivot tuples. It adds an extra
> heap TID from the leaf level when truncating away non-distinguishing
> attributes during
On Mon, Jul 2, 2018 at 9:28 AM, Peter Geoghegan wrote:
>> Execution time of last "VACUUM test;" command on my notebook was:
>>
>> with bulk deletion: 1.6 s;
>> with Quick Vacuum Strategy: 5.2 s;
>> with Quick Vacuum Strategy & TID sorting: 0.6 s.
>
ng any index at all, which Tom referred
to in passing. That's why I understand his remarks in this way.
--
Peter Geoghegan
assing the lowest heap TID in the list of TIDs to kill to
_bt_search() and _bt_binsrch(). You might have to work through several
extra B-Tree leaf pages per bttargetdelete() call without this (you'll
move right multiple times within bttargetdelete()).
--
Peter Geoghegan
BTreeTupleGetNAtts() assertions.
This isn't a correctness issue, and the extra overhead of unneeded
truncation should be negligible, but what we have now seems confusing
to me.
--
Peter Geoghegan
0001-Don-t-truncate-away-non-key-attributes-for-leftmost-.patch
Description: Binary data
l
> that using wall-clock time is the right way to solve that problem
> because it leads to edge effects
I agree that wall-clock time is a bad approach, actually. If we were
to use wall-clock time, we'd only do so because it can be used to
discriminate against a thing that we actually care about in an
approximate, indirect way. If there is a more direct way of
identifying correlated accesses, which I gather that there is, then we
should probably use it.
--
Peter Geoghegan
32 kB of memory left after that
>> (including, if it went below 0), then we bump availMem back up to 32 kB. So
>> we'd always reserve 32 kB to hold the tuples, even if that means that we
>> exceed 'work_mem' slightly.
>
> Sounds very reasonable.
+1
--
Peter Geoghegan
e minimum additional
amount of memory should be big enough that you have some chance of
using all 1024 SortTuples (the whole memtuples array).
--
Peter Geoghegan
1 - 100 of 3069 matches
Mail list logo
