aio: Add WARNING result status

anarazel · anarazel · commit ef64fe26bad9 · 2025-03-30T16:27:10.000-04:00
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
diff --git 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
@@ -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
@@ -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
@@ -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.
  *

Original file line number	Diff line number	Diff line change
`@@ -83,6 +83,7 @@ pgaio_io_register_callbacks(PgAioHandle *ioh, PgAioHandleCallbackID cb_id,`
`83`	`83`	`{`
`84`	`84`	`const PgAioHandleCallbacksEntry *ce = &aio_handle_cbs[cb_id];`
`85`	`85`
	`86`	`+ Assert(cb_id <= PGAIO_HCB_MAX);`
`86`	`87`	`if (cb_id >= lengthof(aio_handle_cbs))`
`87`	`88`	`elog(ERROR, "callback %d is out of range", cb_id);`
`88`	`89`	`if (aio_handle_cbs[cb_id].cb->complete_shared == NULL &&`