Failed to request an autovacuum work-item in silence
Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
report_autovac_workitem_request_failure.patchapplication/octet-stream; name=report_autovac_workitem_request_failure.patchDownload
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 75c2362..f322bd6 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -3199,6 +3199,7 @@ AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
BlockNumber blkno)
{
int i;
+ bool requested = false;
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
@@ -3218,11 +3219,17 @@ AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
workitem->avw_database = MyDatabaseId;
workitem->avw_relation = relationId;
workitem->avw_blockNumber = blkno;
+ requested = true;
/* done */
break;
}
+ if (!requested)
+ ereport(LOG, (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg("could not request an autovacuum work item %d for OID %u",
+ type, relationId)));
+
LWLockRelease(AutovacuumLock);
}
Em ter, 23 de jan de 2018 às 03:36, Masahiko Sawada <sawada.mshk@gmail.com>
escreveu:
Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.
Seems reasonable but maybe you can use the word "worker" instead of "work
item" for report message.
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On Tue, Jan 23, 2018 at 8:03 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
Em ter, 23 de jan de 2018 às 03:36, Masahiko Sawada <sawada.mshk@gmail.com>
escreveu:Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.Seems reasonable but maybe you can use the word "worker" instead of "work
item" for report message.
Thank you for the comment.
Or can we use the word "work-item" since the commit log and source
code use this word?
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Tue, Jan 23, 2018 at 11:44 PM, Masahiko Sawada <sawada.mshk@gmail.com>
wrote:
On Tue, Jan 23, 2018 at 8:03 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Em ter, 23 de jan de 2018 às 03:36, Masahiko Sawada <
sawada.mshk@gmail.com>
escreveu:
Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.Seems reasonable but maybe you can use the word "worker" instead of
"work
item" for report message.
Thank you for the comment.
Or can we use the word "work-item" since the commit log and source
code use this word?
You're correct, I misunderstood it thinking about autovacuum workers and
not the internal workitem array.
As NUM_WORKITEMS is fixed in 256 we don't have any real feedback if in a
real workload this can send a lot of messages to log, so:
1) maybe invent a new GUC to control if we need or not to send this info to
log
2) change elevel for DEBUG1
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On Wed, Jan 24, 2018 at 12:31 PM, Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
On Tue, Jan 23, 2018 at 11:44 PM, Masahiko Sawada <sawada.mshk@gmail.com>
wrote:
On Tue, Jan 23, 2018 at 8:03 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Em ter, 23 de jan de 2018 às 03:36, Masahiko Sawada <
sawada.mshk@gmail.com>
escreveu:
Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.Seems reasonable but maybe you can use the word "worker" instead of
"work
item" for report message.
Thank you for the comment.
Or can we use the word "work-item" since the commit log and source
code use this word?You're correct, I misunderstood it thinking about autovacuum workers and
not the internal workitem array.
As NUM_WORKITEMS is fixed in 256 we don't have any real feedback if in a
real workload this can send a lot of messages to log, so:
1) maybe invent a new GUC to control if we need or not to send this info
to log
2) change elevel for DEBUG1
Looking better for the autovacuum code your patch will work just for BRIN
request "brin_summarize_range", correct?
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On Thu, Jan 25, 2018 at 12:14 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
On Wed, Jan 24, 2018 at 12:31 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Tue, Jan 23, 2018 at 11:44 PM, Masahiko Sawada <sawada.mshk@gmail.com>
wrote:On Tue, Jan 23, 2018 at 8:03 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Em ter, 23 de jan de 2018 às 03:36, Masahiko Sawada
<sawada.mshk@gmail.com>
escreveu:Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.Seems reasonable but maybe you can use the word "worker" instead of
"work
item" for report message.Thank you for the comment.
Or can we use the word "work-item" since the commit log and source
code use this word?You're correct, I misunderstood it thinking about autovacuum workers and
not the internal workitem array.As NUM_WORKITEMS is fixed in 256 we don't have any real feedback if in a
real workload this can send a lot of messages to log, so:
1) maybe invent a new GUC to control if we need or not to send this info
to log
2) change elevel for DEBUG1
Hmm, I'd rather think to log the message at LOG level and to have a
new GUC to control the size of maximum of work-items array . I think
we should always notify users that work-item couldn't get added
because it can become a potential performance problem. Also it might
lead other problems once we have other types of work-item. In the log
message, we can hint for user that you might want to increase maximum
of work-items array.
Looking better for the autovacuum code your patch will work just for BRIN
request "brin_summarize_range", correct?
Correct.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Thu, Jan 25, 2018 at 10:41 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Thu, Jan 25, 2018 at 12:14 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Wed, Jan 24, 2018 at 12:31 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Tue, Jan 23, 2018 at 11:44 PM, Masahiko Sawada <sawada.mshk@gmail.com>
wrote:On Tue, Jan 23, 2018 at 8:03 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Em ter, 23 de jan de 2018 às 03:36, Masahiko Sawada
<sawada.mshk@gmail.com>
escreveu:Hi all,
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.Seems reasonable but maybe you can use the word "worker" instead of
"work
item" for report message.Thank you for the comment.
Or can we use the word "work-item" since the commit log and source
code use this word?You're correct, I misunderstood it thinking about autovacuum workers and
not the internal workitem array.As NUM_WORKITEMS is fixed in 256 we don't have any real feedback if in a
real workload this can send a lot of messages to log, so:
1) maybe invent a new GUC to control if we need or not to send this info
to log
2) change elevel for DEBUG1Hmm, I'd rather think to log the message at LOG level and to have a
new GUC to control the size of maximum of work-items array . I think
we should always notify users that work-item couldn't get added
because it can become a potential performance problem. Also it might
lead other problems once we have other types of work-item. In the log
message, we can hint for user that you might want to increase maximum
of work-items array.Looking better for the autovacuum code your patch will work just for BRIN
request "brin_summarize_range", correct?Correct.
I've added this to next CF so as not to forget.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Hi,
On 2018-01-23 14:35:42 +0900, Masahiko Sawada wrote:
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.
Alvaro?
- Andres
Masahiko Sawada wrote:
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.
Hmm, yeah.
I think it would be better to return false to caller, and have them
report the failure. Then they're in a better position to place an
accurate log message -- for instance indicating the relation name rather
than just OID, and specify work item type rather than some weird
integer. (I think the ereport() should definitely be *outside* the
locked section, in any case.)
I'm inclined to change the API of AutoVacuumRequestWork even in branch
10. Since this stuff is not nicely extensible (c.f. perform_work_item),
it doesn't seem likely that any outside code is calling it yet. Once we
have hooks to register worker functions and stuff, it'll become more of
a problem.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Thank you the comment.
On Fri, Mar 2, 2018 at 4:18 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Masahiko Sawada wrote:
While reading the code, I realized that the requesting an autovacuum
work-item could fail in silence if work-item array is full. So the
users cannot realize that work-item is never performed.
AutoVacuumRequestWork() seems to behave so from the initial
implementation but is there any reason of such behavior? It seems to
me that it can be a problem even now that there is only one kind of
work-item. Attached patch for fixing it.Hmm, yeah.
I think it would be better to return false to caller, and have them
report the failure. Then they're in a better position to place an
accurate log message -- for instance indicating the relation name rather
than just OID, and specify work item type rather than some weird
integer. (I think the ereport() should definitely be *outside* the
locked section, in any case.)
Agreed. Attached an updated patch.
I'm inclined to change the API of AutoVacuumRequestWork even in branch
10. Since this stuff is not nicely extensible (c.f. perform_work_item),
it doesn't seem likely that any outside code is calling it yet. Once we
have hooks to register worker functions and stuff, it'll become more of
a problem.
+1
Since this API cannot be execute actually other than brin
summarization I think we can change it in branch 10.
It's good if autovacuum work-items can be added by extensions and so
on. Also I'm inclined to change the autovacuum launcher so that it can
launcher new worker for performing work-item or to allow requests to
specify the execution timing (before or after vacuum), it's for PG12
item though.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
report_autovac_workitem_request_failure_v2.patchapplication/octet-stream; name=report_autovac_workitem_request_failure_v2.patchDownload
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 21f5e2c..59647fb 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -343,6 +343,7 @@ static void perform_work_item(AutoVacuumWorkItem *workitem);
static void autovac_report_activity(autovac_table *tab);
static void autovac_report_workitem(AutoVacuumWorkItem *workitem,
const char *nspname, const char *relname);
+static const char *autovac_get_workitem_name(AutoVacuumWorkItemType type);
static void av_sighup_handler(SIGNAL_ARGS);
static void avl_sigusr2_handler(SIGNAL_ARGS);
static void avl_sigterm_handler(SIGNAL_ARGS);
@@ -3207,11 +3208,12 @@ AutoVacuumingActive(void)
/*
* Request one work item to the next autovacuum run processing our database.
*/
-void
+bool
AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
BlockNumber blkno)
{
int i;
+ bool requested = false;
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
@@ -3231,12 +3233,41 @@ AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
workitem->avw_database = MyDatabaseId;
workitem->avw_relation = relationId;
workitem->avw_blockNumber = blkno;
+ requested = true;
/* done */
break;
}
LWLockRelease(AutovacuumLock);
+
+ if (!requested)
+ ereport(LOG, (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg("could not request an autovacuum work item \"%s\" for \"%s\"",
+ autovac_get_workitem_name(type), get_rel_name(relationId))));
+
+ return requested;
+}
+
+/*
+ * autovac_get_workitem_name
+ *
+ * Convert AutoVacuumWorkerItemType to string.
+ */
+static const char *
+autovac_get_workitem_name(AutoVacuumWorkItemType type)
+{
+ const char *workitem_name = "UnknownWorkItem";
+
+ switch (type)
+ {
+ case AVW_BRINSummarizeRange:
+ workitem_name = "BrinSummarizeRange";
+ break;
+ /* no default case, so that compiler will warn */
+ }
+
+ return workitem_name;
}
/*
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index 18cff54..96752ca 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -71,7 +71,7 @@ extern void AutovacuumWorkerIAm(void);
extern void AutovacuumLauncherIAm(void);
#endif
-extern void AutoVacuumRequestWork(AutoVacuumWorkItemType type,
+extern bool AutoVacuumRequestWork(AutoVacuumWorkItemType type,
Oid relationId, BlockNumber blkno);
/* shared memory stuff */
Hello,
The patch applies and compiles cleanly, tests pass. The code is working
as well. I was able to check it by simply creating a BRIN index and
filling the table with data forcing the index to request lots of work items:
create table test (a serial, b text);
create index on test using brin (a) with (pages_per_range=1,
autosummarize=true);
insert into test select i, repeat(md5(random()::text), 30) from
generate_series(1, 3000) i;
LOG: could not request an autovacuum work item "BrinSummarizeRange" for
"test_a_idx"
LOG: could not request an autovacuum work item "BrinSummarizeRange" for
"test_a_idx"
...
Just couple remarks. I would rename 'requested' variable in
AutoVacuumRequestWork() func to something like 'success' or 'result'.
Because request is something caller does. And I would also rephrase log
message as follows:
request for autovacuum work item "%s" for "%s" failed
Thanks!
--
Ildar Musin
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Hi
I was thinking that the BRIN code requesting the workitem would print
the error message based on the return value. There is no point to
returning a boolean indicator if the caller isn't going to do anything
with it ... This means you don't need to convert the type to string in
autovacuum.c (which would defeat attempts at generalizing this code).
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Thank you for reviewing!
On Thu, Mar 8, 2018 at 6:07 PM, Ildar Musin <i.musin@postgrespro.ru> wrote:
Just couple remarks. I would rename 'requested' variable in
AutoVacuumRequestWork() func to something like 'success' or 'result'.
Because request is something caller does. And I would also rephrase log
message as follows:request for autovacuum work item "%s" for "%s" failed
Agreed.
On Thu, Mar 8, 2018 at 10:46 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Hi
I was thinking that the BRIN code requesting the workitem would print
the error message based on the return value. There is no point to
returning a boolean indicator if the caller isn't going to do anything
with it ... This means you don't need to convert the type to string in
autovacuum.c (which would defeat attempts at generalizing this code).
Agreed.
Attached an updated patch. Please review it.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
report_autovac_workitem_request_failure_v3.patchtext/x-patch; charset=US-ASCII; name=report_autovac_workitem_request_failure_v3.patchDownload
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 68b3371..6897b7f 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -33,6 +33,7 @@
#include "utils/index_selfuncs.h"
#include "utils/memutils.h"
#include "utils/rel.h"
+#include "utils/lsyscache.h"
/*
@@ -187,9 +188,19 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
brinGetTupleForHeapBlock(revmap, lastPageRange, &buf, &off,
NULL, BUFFER_LOCK_SHARE, NULL);
if (!lastPageTuple)
- AutoVacuumRequestWork(AVW_BRINSummarizeRange,
- RelationGetRelid(idxRel),
- lastPageRange);
+ {
+ bool requested;
+
+ requested = AutoVacuumRequestWork(AVW_BRINSummarizeRange,
+ RelationGetRelid(idxRel),
+ lastPageRange);
+
+ if (!requested)
+ ereport(LOG, (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ errmsg("request for autovacuum work item BrinSummarizeRange for \"%s\" failed",
+ RelationGetRelationName(idxRel))));
+ }
+
else
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 21f5e2c..fa4b42c 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -343,6 +343,7 @@ static void perform_work_item(AutoVacuumWorkItem *workitem);
static void autovac_report_activity(autovac_table *tab);
static void autovac_report_workitem(AutoVacuumWorkItem *workitem,
const char *nspname, const char *relname);
+static const char *autovac_get_workitem_name(AutoVacuumWorkItemType type);
static void av_sighup_handler(SIGNAL_ARGS);
static void avl_sigusr2_handler(SIGNAL_ARGS);
static void avl_sigterm_handler(SIGNAL_ARGS);
@@ -3207,11 +3208,12 @@ AutoVacuumingActive(void)
/*
* Request one work item to the next autovacuum run processing our database.
*/
-void
+bool
AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
BlockNumber blkno)
{
int i;
+ bool result = false;
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
@@ -3231,12 +3233,15 @@ AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
workitem->avw_database = MyDatabaseId;
workitem->avw_relation = relationId;
workitem->avw_blockNumber = blkno;
+ result = true;
/* done */
break;
}
LWLockRelease(AutovacuumLock);
+
+ return result;
}
/*
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index 18cff54..96752ca 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -71,7 +71,7 @@ extern void AutovacuumWorkerIAm(void);
extern void AutovacuumLauncherIAm(void);
#endif
-extern void AutoVacuumRequestWork(AutoVacuumWorkItemType type,
+extern bool AutoVacuumRequestWork(AutoVacuumWorkItemType type,
Oid relationId, BlockNumber blkno);
/* shared memory stuff */
Hi,
autovac_get_workitem_name() declaration seems redundant and should be
removed. The same thing with including "utils/lsyscache.h" in brin.c.
The 'requested' variable in brininsert() I would again rename to
something like 'success' because a work item is requested anyway but
what matters is whether the request was satisfied/successful.
Except for those two points everything is fine and works as expected.
--
Ildar Musin
i.musin@postgrespro.ru
Hello,
Ildar Musin wrote:
autovac_get_workitem_name() declaration seems redundant and should be
removed. The same thing with including "utils/lsyscache.h" in brin.c.The 'requested' variable in brininsert() I would again rename to something
like 'success' because a work item is requested anyway but what matters is
whether the request was satisfied/successful.
Thanks, I pushed this. I agree with your comments; so I changed
'requested' to 'recorded' and removed those lines. I also reworded the
log message:
ereport(LOG,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("request for BRIN range summarization for index \"%s\" page %u was not recorded",
RelationGetRelationName(idxRel),
lastPageRange)));
And added a paragraph to the docs explaining this situation.
Now I'm wondering what will we tell users to do if they get this message
too frequently. Neither of the obvious options (1. changing the index's
pages_per_range to a larger value; 2. making autovacuum more frequent
somehow) seem terribly useful.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 15, 2018 at 12:06 AM, Alvaro Herrera
<alvherre@alvh.no-ip.org> wrote:
Hello,
Ildar Musin wrote:
autovac_get_workitem_name() declaration seems redundant and should be
removed. The same thing with including "utils/lsyscache.h" in brin.c.The 'requested' variable in brininsert() I would again rename to something
like 'success' because a work item is requested anyway but what matters is
whether the request was satisfied/successful.Thanks, I pushed this. I agree with your comments; so I changed
'requested' to 'recorded' and removed those lines.
Thank you!
I also reworded the
log message:ereport(LOG,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("request for BRIN range summarization for index \"%s\" page %u was not recorded",
RelationGetRelationName(idxRel),
lastPageRange)));And added a paragraph to the docs explaining this situation.
Now I'm wondering what will we tell users to do if they get this message
too frequently. Neither of the obvious options (1. changing the index's
pages_per_range to a larger value; 2. making autovacuum more frequent
somehow) seem terribly useful.
Or telling users to call brin_summarize_range() manually?
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Masahiko Sawada wrote:
On Thu, Mar 15, 2018 at 12:06 AM, Alvaro Herrera
<alvherre@alvh.no-ip.org> wrote:
Now I'm wondering what will we tell users to do if they get this message
too frequently. Neither of the obvious options (1. changing the index's
pages_per_range to a larger value; 2. making autovacuum more frequent
somehow) seem terribly useful.Or telling users to call brin_summarize_range() manually?
Yeah, they can do that to fix the situation with each unsummarized
range, but that won't silence the log message ... oh! Unless you call it
to summarize the range ahead of time -- I think that should fix it. Is
that what you were thinking?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 15, 2018 at 7:35 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Masahiko Sawada wrote:
On Thu, Mar 15, 2018 at 12:06 AM, Alvaro Herrera
<alvherre@alvh.no-ip.org> wrote:Now I'm wondering what will we tell users to do if they get this message
too frequently. Neither of the obvious options (1. changing the index's
pages_per_range to a larger value; 2. making autovacuum more frequent
somehow) seem terribly useful.Or telling users to call brin_summarize_range() manually?
Yeah, they can do that to fix the situation with each unsummarized
range, but that won't silence the log message ... oh! Unless you call it
to summarize the range ahead of time -- I think that should fix it. Is
that what you were thinking?
Yes. Or with this situation since the multiple work-item for the same
index but different block number might be listed the calling
brin_summarize_new_values() manually would rather fix the situation
more properly?
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center