Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e10b7d8

Browse filesBrowse files
arssherarssher
committed
Streamline error handling in spawn partitions worker bgw.
Avoid intercepting error in it. Standard bgw error handling code should be enough: it emits error and exits cleanly.
1 parent 3fa5d08 commit e10b7d8
Copy full SHA for e10b7d8

File tree

3 files changed

+99
-129
lines changed
Filter options

3 files changed

+99
-129
lines changed

‎src/include/partition_creation.h

Copy file name to clipboardExpand all lines: src/include/partition_creation.h
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424

2525
/* Create RANGE partitions to store some value */
2626
Oid create_partitions_for_value(Oid relid, Datum value, Oid value_type);
27-
Oid create_partitions_for_value_internal(Oid relid, Datum value, Oid value_type,
28-
bool is_background_worker);
27+
Oid create_partitions_for_value_internal(Oid relid, Datum value, Oid value_type);
2928

3029

3130
/* Create one RANGE partition */

‎src/partition_creation.c

Copy file name to clipboardExpand all lines: src/partition_creation.c
+88-117Lines changed: 88 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,7 @@ create_partitions_for_value(Oid relid, Datum value, Oid value_type)
315315
elog(DEBUG2, "create_partitions(): chose backend [%u]", MyProcPid);
316316
last_partition = create_partitions_for_value_internal(relid,
317317
value,
318-
value_type,
319-
false); /* backend */
318+
value_type);
320319
}
321320
}
322321
else
@@ -348,147 +347,119 @@ create_partitions_for_value(Oid relid, Datum value, Oid value_type)
348347
* use create_partitions_for_value() instead.
349348
*/
350349
Oid
351-
create_partitions_for_value_internal(Oid relid, Datum value, Oid value_type,
352-
bool is_background_worker)
350+
create_partitions_for_value_internal(Oid relid, Datum value, Oid value_type)
353351
{
354352
MemoryContext old_mcxt = CurrentMemoryContext;
355353
Oid partid = InvalidOid; /* last created partition (or InvalidOid) */
354+
Datum values[Natts_pathman_config];
355+
bool isnull[Natts_pathman_config];
356356

357-
PG_TRY();
357+
/* Get both PartRelationInfo & PATHMAN_CONFIG contents for this relation */
358+
if (pathman_config_contains_relation(relid, values, isnull, NULL, NULL))
358359
{
359-
Datum values[Natts_pathman_config];
360-
bool isnull[Natts_pathman_config];
360+
PartRelationInfo *prel;
361+
LockAcquireResult lock_result; /* could we lock the parent? */
362+
Oid base_bound_type; /* base type of prel->ev_type */
363+
Oid base_value_type; /* base type of value_type */
361364

362-
/* Get both PartRelationInfo & PATHMAN_CONFIG contents for this relation */
363-
if (pathman_config_contains_relation(relid, values, isnull, NULL, NULL))
364-
{
365-
PartRelationInfo *prel;
366-
LockAcquireResult lock_result; /* could we lock the parent? */
367-
Oid base_bound_type; /* base type of prel->ev_type */
368-
Oid base_value_type; /* base type of value_type */
369-
370-
/* Prevent modifications of partitioning scheme */
371-
lock_result = xact_lock_rel(relid, ShareUpdateExclusiveLock, false);
365+
/* Prevent modifications of partitioning scheme */
366+
lock_result = xact_lock_rel(relid, ShareUpdateExclusiveLock, false);
372367

373-
/* Fetch PartRelationInfo by 'relid' */
374-
prel = get_pathman_relation_info(relid);
375-
shout_if_prel_is_invalid(relid, prel, PT_RANGE);
368+
/* Fetch PartRelationInfo by 'relid' */
369+
prel = get_pathman_relation_info(relid);
370+
shout_if_prel_is_invalid(relid, prel, PT_RANGE);
376371

377-
/* Fetch base types of prel->ev_type & value_type */
378-
base_bound_type = getBaseType(prel->ev_type);
379-
base_value_type = getBaseType(value_type);
372+
/* Fetch base types of prel->ev_type & value_type */
373+
base_bound_type = getBaseType(prel->ev_type);
374+
base_value_type = getBaseType(value_type);
380375

381-
/*
382-
* Search for a suitable partition if we didn't hold it,
383-
* since somebody might have just created it for us.
384-
*
385-
* If the table is locked, it means that we've
386-
* already failed to find a suitable partition
387-
* and called this function to do the job.
388-
*/
389-
Assert(lock_result != LOCKACQUIRE_NOT_AVAIL);
390-
if (lock_result == LOCKACQUIRE_OK)
391-
{
392-
Oid *parts;
393-
int nparts;
394-
395-
/* Search for matching partitions */
396-
parts = find_partitions_for_value(value, value_type, prel, &nparts);
397-
398-
/* Shout if there's more than one */
399-
if (nparts > 1)
400-
elog(ERROR, ERR_PART_ATTR_MULTIPLE);
401-
402-
/* It seems that we got a partition! */
403-
else if (nparts == 1)
404-
{
405-
/* Unlock the parent (we're not going to spawn) */
406-
UnlockRelationOid(relid, ShareUpdateExclusiveLock);
376+
/*
377+
* Search for a suitable partition if we didn't hold it,
378+
* since somebody might have just created it for us.
379+
*
380+
* If the table is locked, it means that we've
381+
* already failed to find a suitable partition
382+
* and called this function to do the job.
383+
*/
384+
Assert(lock_result != LOCKACQUIRE_NOT_AVAIL);
385+
if (lock_result == LOCKACQUIRE_OK)
386+
{
387+
Oid *parts;
388+
int nparts;
407389

408-
/* Simply return the suitable partition */
409-
partid = parts[0];
410-
}
390+
/* Search for matching partitions */
391+
parts = find_partitions_for_value(value, value_type, prel, &nparts);
411392

412-
/* Don't forget to free */
413-
pfree(parts);
414-
}
393+
/* Shout if there's more than one */
394+
if (nparts > 1)
395+
elog(ERROR, ERR_PART_ATTR_MULTIPLE);
415396

416-
/* Else spawn a new one (we hold a lock on the parent) */
417-
if (partid == InvalidOid)
397+
/* It seems that we got a partition! */
398+
else if (nparts == 1)
418399
{
419-
RangeEntry *ranges = PrelGetRangesArray(prel);
420-
Bound bound_min, /* absolute MIN */
421-
bound_max; /* absolute MAX */
400+
/* Unlock the parent (we're not going to spawn) */
401+
UnlockRelationOid(relid, ShareUpdateExclusiveLock);
422402

423-
Oid interval_type = InvalidOid;
424-
Datum interval_binary, /* assigned 'width' of one partition */
425-
interval_text;
403+
/* Simply return the suitable partition */
404+
partid = parts[0];
405+
}
426406

427-
/* Copy datums in order to protect them from cache invalidation */
428-
bound_min = CopyBound(&ranges[0].min,
429-
prel->ev_byval,
430-
prel->ev_len);
407+
/* Don't forget to free */
408+
pfree(parts);
409+
}
431410

432-
bound_max = CopyBound(&ranges[PrelLastChild(prel)].max,
433-
prel->ev_byval,
434-
prel->ev_len);
411+
/* Else spawn a new one (we hold a lock on the parent) */
412+
if (partid == InvalidOid)
413+
{
414+
RangeEntry *ranges = PrelGetRangesArray(prel);
415+
Bound bound_min, /* absolute MIN */
416+
bound_max; /* absolute MAX */
435417

436-
/* Check if interval is set */
437-
if (isnull[Anum_pathman_config_range_interval - 1])
438-
{
439-
ereport(ERROR,
440-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
441-
errmsg("cannot spawn new partition for key '%s'",
442-
datum_to_cstring(value, value_type)),
443-
errdetail("default range interval is NULL")));
444-
}
418+
Oid interval_type = InvalidOid;
419+
Datum interval_binary, /* assigned 'width' of one partition */
420+
interval_text;
445421

446-
/* Retrieve interval as TEXT from tuple */
447-
interval_text = values[Anum_pathman_config_range_interval - 1];
422+
/* Copy datums in order to protect them from cache invalidation */
423+
bound_min = CopyBound(&ranges[0].min,
424+
prel->ev_byval,
425+
prel->ev_len);
448426

449-
/* Convert interval to binary representation */
450-
interval_binary = extract_binary_interval_from_text(interval_text,
451-
base_bound_type,
452-
&interval_type);
427+
bound_max = CopyBound(&ranges[PrelLastChild(prel)].max,
428+
prel->ev_byval,
429+
prel->ev_len);
453430

454-
/* At last, spawn partitions to store the value */
455-
partid = spawn_partitions_val(PrelParentRelid(prel),
456-
&bound_min, &bound_max, base_bound_type,
457-
interval_binary, interval_type,
458-
value, base_value_type,
459-
prel->ev_collid);
431+
/* Check if interval is set */
432+
if (isnull[Anum_pathman_config_range_interval - 1])
433+
{
434+
ereport(ERROR,
435+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
436+
errmsg("cannot spawn new partition for key '%s'",
437+
datum_to_cstring(value, value_type)),
438+
errdetail("default range interval is NULL")));
460439
}
461440

462-
/* Don't forget to close 'prel'! */
463-
close_pathman_relation_info(prel);
464-
}
465-
else
466-
elog(ERROR, "table \"%s\" is not partitioned",
467-
get_rel_name_or_relid(relid));
468-
}
469-
PG_CATCH();
470-
{
471-
ErrorData *error;
441+
/* Retrieve interval as TEXT from tuple */
442+
interval_text = values[Anum_pathman_config_range_interval - 1];
472443

473-
/* Simply rethrow ERROR if we're in backend */
474-
if (!is_background_worker)
475-
PG_RE_THROW();
444+
/* Convert interval to binary representation */
445+
interval_binary = extract_binary_interval_from_text(interval_text,
446+
base_bound_type,
447+
&interval_type);
476448

477-
/* Switch to the original context & copy edata */
478-
MemoryContextSwitchTo(old_mcxt);
479-
error = CopyErrorData();
480-
FlushErrorState();
481-
482-
/* Produce log message if we're in BGW */
483-
elog(LOG,
484-
CppAsString(create_partitions_for_value_internal) ": %s [%u]",
485-
error->message,
486-
MyProcPid);
449+
/* At last, spawn partitions to store the value */
450+
partid = spawn_partitions_val(PrelParentRelid(prel),
451+
&bound_min, &bound_max, base_bound_type,
452+
interval_binary, interval_type,
453+
value, base_value_type,
454+
prel->ev_collid);
455+
}
487456

488-
/* Reset 'partid' in case of error */
489-
partid = InvalidOid;
457+
/* Don't forget to close 'prel'! */
458+
close_pathman_relation_info(prel);
490459
}
491-
PG_END_TRY();
460+
else
461+
elog(ERROR, "table \"%s\" is not partitioned",
462+
get_rel_name_or_relid(relid));
492463

493464
return partid;
494465
}

‎src/pathman_workers.c

Copy file name to clipboardExpand all lines: src/pathman_workers.c
+10-10Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ bgw_main_spawn_partitions(Datum main_arg)
364364
dsm_segment *segment;
365365
SpawnPartitionArgs *args;
366366
Datum value;
367+
Oid result;
367368

368369
/* Establish signal handlers before unblocking signals. */
369370
pqsignal(SIGTERM, handle_sigterm);
@@ -415,18 +416,17 @@ bgw_main_spawn_partitions(Datum main_arg)
415416
DebugPrintDatum(value, args->value_type), MyProcPid);
416417
#endif
417418

418-
/* Create partitions and save the Oid of the last one */
419-
args->result = create_partitions_for_value_internal(args->partitioned_table,
420-
value, /* unpacked Datum */
421-
args->value_type,
422-
true); /* background woker */
419+
/*
420+
* Create partitions and save the Oid of the last one.
421+
* If we fail here, args->result is 0 since it is zeroed on initialization.
422+
*/
423+
result = create_partitions_for_value_internal(args->partitioned_table,
424+
value, /* unpacked Datum */
425+
args->value_type);
423426

424427
/* Finish transaction in an appropriate way */
425-
if (args->result == InvalidOid)
426-
AbortCurrentTransaction();
427-
else
428-
CommitTransactionCommand();
429-
428+
CommitTransactionCommand();
429+
args->result = result;
430430
dsm_detach(segment);
431431
}
432432

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.