diff options
author | Andres Freund <andres@anarazel.de> | 2025-03-30 16:10:51 -0400 |
---|---|---|
committer | Andres Freund <andres@anarazel.de> | 2025-03-30 16:27:10 -0400 |
commit | ef64fe26bad92a7b8425767cdbbe8b946d4637f0 (patch) | |
tree | a76ed73de21d69f9e24fe590e6b207ddfd2b31a9 | |
parent | d445990adc419f435360f0dcd91c05c082f5fa3f (diff) | |
download | postgresql-ef64fe26bad92a7b8425767cdbbe8b946d4637f0.tar.gz postgresql-ef64fe26bad92a7b8425767cdbbe8b946d4637f0.zip |
aio: Add WARNING result status
If an IO succeeds, but issues a warning, e.g. due to a page verification
failure with zero_damaged_pages, we want to issue that warning in the context
of the issuer of the IO, not the process that executes the completion (always
the case for worker).
It's already possible for a completion callback to report a custom error
message, we just didn't have a result status that allowed a user of AIO to
know that a warning should be emitted even though the IO request succeeded.
All that's needed for that is a dedicated PGAIO_RS_ value.
Previously there were not enough bits in PgAioResult.id for the new
value. Increase. While at that, add defines for the amount of bits and static
asserts to check that the widths are appropriate.
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250329212929.a6.nmisch@google.com
-rw-r--r-- | src/backend/storage/aio/aio.c | 2 | ||||
-rw-r--r-- | src/backend/storage/aio/aio_callback.c | 1 | ||||
-rw-r--r-- | src/include/storage/aio.h | 6 | ||||
-rw-r--r-- | src/include/storage/aio_types.h | 26 |
4 files changed, 29 insertions, 6 deletions
diff --git a/src/backend/storage/aio/aio.c b/src/backend/storage/aio/aio.c index 91e76113412..e3ed087e8a2 100644 --- a/src/backend/storage/aio/aio.c +++ b/src/backend/storage/aio/aio.c @@ -839,6 +839,8 @@ pgaio_result_status_string(PgAioResultStatus rs) return "UNKNOWN"; case PGAIO_RS_OK: return "OK"; + case PGAIO_RS_WARNING: + return "WARNING"; case PGAIO_RS_PARTIAL: return "PARTIAL"; case PGAIO_RS_ERROR: diff --git a/src/backend/storage/aio/aio_callback.c b/src/backend/storage/aio/aio_callback.c index 53db6e194af..b00b6bc1025 100644 --- a/src/backend/storage/aio/aio_callback.c +++ b/src/backend/storage/aio/aio_callback.c @@ -83,6 +83,7 @@ pgaio_io_register_callbacks(PgAioHandle *ioh, PgAioHandleCallbackID cb_id, { const PgAioHandleCallbacksEntry *ce = &aio_handle_cbs[cb_id]; + Assert(cb_id <= PGAIO_HCB_MAX); if (cb_id >= lengthof(aio_handle_cbs)) elog(ERROR, "callback %d is out of range", cb_id); if (aio_handle_cbs[cb_id].cb->complete_shared == NULL && diff --git a/src/include/storage/aio.h b/src/include/storage/aio.h index c6bf7c4ccf5..4ab4b05145a 100644 --- a/src/include/storage/aio.h +++ b/src/include/storage/aio.h @@ -191,11 +191,15 @@ struct PgAioTargetInfo */ typedef enum PgAioHandleCallbackID { - PGAIO_HCB_INVALID, + PGAIO_HCB_INVALID = 0, PGAIO_HCB_MD_READV, } PgAioHandleCallbackID; +#define PGAIO_HCB_MAX PGAIO_HCB_MD_READV +StaticAssertDecl(PGAIO_HCB_MAX <= (1 << PGAIO_RESULT_ID_BITS), + "PGAIO_HCB_MAX is too big for PGAIO_RESULT_ID_BITS"); + typedef void (*PgAioHandleCallbackStage) (PgAioHandle *ioh, uint8 cb_flags); typedef PgAioResult (*PgAioHandleCallbackComplete) (PgAioHandle *ioh, PgAioResult prior_result, uint8 cb_flags); diff --git a/src/include/storage/aio_types.h b/src/include/storage/aio_types.h index debe8163d4e..18183366077 100644 --- a/src/include/storage/aio_types.h +++ b/src/include/storage/aio_types.h @@ -79,32 +79,48 @@ typedef enum PgAioResultStatus { PGAIO_RS_UNKNOWN, /* not yet completed / uninitialized */ PGAIO_RS_OK, - PGAIO_RS_PARTIAL, /* did not fully succeed, but no error */ - PGAIO_RS_ERROR, + PGAIO_RS_PARTIAL, /* did not fully succeed, no warning/error */ + PGAIO_RS_WARNING, /* [partially] succeeded, with a warning */ + PGAIO_RS_ERROR, /* failed entirely */ } PgAioResultStatus; /* * Result of IO operation, visible only to the initiator of IO. + * + * We need to be careful about the size of PgAioResult, as it is embedded in + * every PgAioHandle, as well as every PgAioReturn. Currently we assume we can + * fit it into one 8 byte value, restricting the space for per-callback error + * data to PGAIO_RESULT_ERROR_BITS. */ +#define PGAIO_RESULT_ID_BITS 6 +#define PGAIO_RESULT_STATUS_BITS 3 +#define PGAIO_RESULT_ERROR_BITS 23 typedef struct PgAioResult { /* * This is of type PgAioHandleCallbackID, but can't use a bitfield of an * enum, because some compilers treat enums as signed. */ - uint32 id:8; + uint32 id:PGAIO_RESULT_ID_BITS; /* of type PgAioResultStatus, see above */ - uint32 status:2; + uint32 status:PGAIO_RESULT_STATUS_BITS; /* meaning defined by callback->error */ - uint32 error_data:22; + uint32 error_data:PGAIO_RESULT_ERROR_BITS; int32 result; } PgAioResult; +StaticAssertDecl(PGAIO_RESULT_ID_BITS + + PGAIO_RESULT_STATUS_BITS + + PGAIO_RESULT_ERROR_BITS == 32, + "PgAioResult bits divided up incorrectly"); +StaticAssertDecl(sizeof(PgAioResult) == 8, + "PgAioResult has unexpected size"); + /* * Combination of PgAioResult with minimal metadata about the IO. * |