On Wed, Feb 1, 2017 at 7:55 PM, Claudio Freire <klaussfre...@gmail.com> wrote:
> On Wed, Feb 1, 2017 at 6:13 PM, Masahiko Sawada <sawada.m...@gmail.com> wrote:
>> On Wed, Feb 1, 2017 at 10:02 PM, Claudio Freire <klaussfre...@gmail.com>
>> wrote:
>>> On Wed, Feb 1, 2017 at 5:47 PM, Masahiko Sawada <sawada.m...@gmail.com>
>>> wrote:
>>>> Thank you for updating the patch.
>>>>
>>>> Whole patch looks good to me except for the following one comment.
>>>> This is the final comment from me.
>>>>
>>>> /*
>>>> * lazy_tid_reaped() -- is a particular tid deletable?
>>>> *
>>>> * This has the right signature to be an IndexBulkDeleteCallback.
>>>> *
>>>> * Assumes dead_tuples array is in sorted order.
>>>> */
>>>> static bool
>>>> lazy_tid_reaped(ItemPointer itemptr, void *state)
>>>> {
>>>> LVRelStats *vacrelstats = (LVRelStats *) state;
>>>>
>>>> You might want to update the comment of lazy_tid_reaped() as well.
>>>
>>> I don't see the mismatch with reality there (if you consider
>>> "dead_tples array" in the proper context, that is, the multiarray).
>>>
>>> What in particular do you find out of sync there?
>>
>> The current lazy_tid_reaped just find a tid from a tid array using
>> bsearch but in your patch lazy_tid_reaped handles multiple tid arrays
>> and processing method become complicated. So I thought it's better to
>> add the description of this function.
>
> Alright, updated with some more remarks that seemed relevant
I just realized I never updated the early free patch after the
multiarray version.
So attached is a patch that frees the multiarray as early as possible
(just after finishing with index bulk deletes, right before doing
index cleanup and attempting truncation).
This should make the possibly big amount of memory available to other
processes for the duration of those tasks, which could be a long time
in some cases.
From f080f8377b7200ae9c2a02abfb0ef0bf6e2d5d69 Mon Sep 17 00:00:00 2001
From: Claudio Freire <klaussfre...@gmail.com>
Date: Tue, 28 Mar 2017 22:40:39 -0300
Subject: [PATCH] Vacuum: free dead tuples array as early as possible
Allow other parts of the system to benefit from the possibly
large amount of memory reserved for dead tuples after they're
no longer necessary.
While the memory would be freed when the command finishes, it
can sometimes be some considerable time between the time vacuum
is done with the array until the command finishes - mostly due
to truncate taking a long time.
---
src/backend/commands/vacuumlazy.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 4a6895c..60a6c18 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -206,6 +206,7 @@ static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks);
static void lazy_record_dead_tuple(LVRelStats *vacrelstats,
ItemPointer itemptr);
static void lazy_clear_dead_tuples(LVRelStats *vacrelstats);
+static void lazy_free_dead_tuples(LVRelStats *vacrelstats);
static bool lazy_tid_reaped(ItemPointer itemptr, void *state);
static inline int vac_cmp_itemptr(const void *left, const void *right);
static bool heap_page_is_all_visible(Relation rel, Buffer buf,
@@ -1358,6 +1359,9 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_PHASE_INDEX_CLEANUP);
+ /* dead tuples no longer needed past this point */
+ lazy_free_dead_tuples(vacrelstats);
+
/* Do post-vacuum cleanup and statistics update for each index */
for (i = 0; i < nindexes; i++)
lazy_cleanup_index(Irel[i], indstats[i], vacrelstats);
@@ -2165,6 +2169,24 @@ lazy_clear_dead_tuples(LVRelStats *vacrelstats)
}
/*
+ * lazy_free_dead_tuples - reset all dead tuples segments
+ * and free all allocated memory
+ */
+static void
+lazy_free_dead_tuples(LVRelStats *vacrelstats)
+{
+ int nseg;
+
+ for (nseg = 0; nseg < vacrelstats->dead_tuples.num_segs; nseg++)
+ pfree(vacrelstats->dead_tuples.dt_segments[nseg].dt_tids);
+ pfree(vacrelstats->dead_tuples.dt_segments);
+ vacrelstats->dead_tuples.last_seg = 0;
+ vacrelstats->dead_tuples.num_segs = 0;
+ vacrelstats->dead_tuples.num_entries = 0;
+ vacrelstats->dead_tuples.dt_segments = NULL;
+}
+
+/*
* lazy_tid_reaped() -- is a particular tid deletable?
*
* This has the right signature to be an IndexBulkDeleteCallback.
--
1.8.4.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
