Thank you for reviewing!
On Thu, Mar 8, 2018 at 6:07 PM, Ildar Musin <[email protected]> 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
<[email protected]> 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
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 */
