rust/neon
1
0
Fork 0
mirror of https://github.com/neondatabase/neon.git synced 2026-01-03 19:42:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

tidy up error handling a bit

Browse Source 
Pass back a suitable 'errno' from the communicator process to the
originating backend in all cases. Usually it's just EIO because we
don't have a good way to map from tonic StatusCodes to libc error
numbers. That's probably good enough; from the original backend's
perspective all errors are IO errors.

In the C code, set libc errno variable before calling ereport(), so
that errcode_for_file_access() works. And once we do that, we can
replace pg_strerror() calls with %m.
This commit is contained in:
Heikki Linnakangas
2025-07-31 15:28:54 +03:00
parent e1df05448c
commit 9871a3f9e7
2 changed files with 59 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
pgxn/neon/communicator/src/worker_process/main_loop.rs
View File

@@ -387,8 +387,9 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
NeonIOResult::RelSize(INVALID_BLOCK_NUMBER)
}
Err(err) => {
// FIXME: Could we map the tonic StatusCode to a libc errno in a more fine-grained way? Or pass the error message to the backend
info!("tonic error: {err:?}");
NeonIOResult::Error(0)
NeonIOResult::Error(libc::EIO)
}
}
}
@@ -426,8 +427,9 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
NeonIOResult::ReadSlruSegment(blocks_count as _)
}
Err(err) => {
// FIXME: Could we map the tonic StatusCode to a libc errno in a more fine-grained way? Or pass the error message to the backend
info!("tonic error: {err:?}");
NeonIOResult::Error(0)
NeonIOResult::Error(libc::EIO)
}
}
}
@@ -435,6 +437,7 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
self.request_nblocks_counters
.inc_by(RequestTypeLabelGroup::from_req(request), req.nblocks as i64);
let req = *req;
// FIXME: handle_request() runs in a separate task already, do we really need to spawn a new one here?
tokio::spawn(async move { self.handle_prefetchv_request(&req).await });
NeonIOResult::PrefetchVLaunched
}
@@ -463,8 +466,9 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
{
Ok(db_size) => NeonIOResult::DbSize(db_size),
Err(err) => {
// FIXME: Could we map the tonic StatusCode to a libc errno in a more fine-grained way? Or pass the error message to the backend
info!("tonic error: {err:?}");
NeonIOResult::Error(0)
NeonIOResult::Error(libc::EIO)
}
}
}
@@ -579,7 +583,7 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
continue;
}
Ok(CacheResult::NotFound(lsn)) => lsn,
Err(_io_error) => return Err(-1), // FIXME errno?
Err(_io_error) => return Err(libc::EIO), // FIXME print the error?
};
cache_misses.push((blkno, not_modified_since, dest, in_progress_guard));
}
@@ -627,7 +631,7 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
resp.pages.len(),
block_numbers.len(),
);
return Err(-1);
return Err(libc::EIO);
}
trace!(
@@ -656,8 +660,9 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
}
}
Err(err) => {
// FIXME: Could we map the tonic StatusCode to a libc errno in a more fine-grained way? Or pass the error message to the backend
info!("tonic error: {err:?}");
return Err(-1);
return Err(libc::EIO);
}
}
Ok(())
@@ -704,7 +709,7 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
resp.pages.len(),
block_numbers.len(),
);
return Err(-1);
return Err(libc::EIO);
}
trace!(
@@ -721,8 +726,9 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
}
}
Err(err) => {
// FIXME: Could we map the tonic StatusCode to a libc errno in a more fine-grained way? Or pass the error message to the backend
info!("tonic error: {err:?}");
return Err(-1);
return Err(libc::EIO);
}
}
Ok(())
@@ -753,7 +759,7 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
continue;
}
Ok(CacheResult::NotFound(lsn)) => lsn,
Err(_io_error) => return Err(-1), // FIXME errno?
Err(_io_error) => return Err(libc::EIO), // FIXME print the error?
};
cache_misses.push((blkno, not_modified_since, in_progress_guard));
}
@@ -795,7 +801,7 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
resp.pages.len(),
block_numbers.len(),
);
return Err(-1);
return Err(libc::EIO);
}
for (page, (blkno, _lsn, _guard)) in resp.pages.into_iter().zip(cache_misses) {
@@ -805,8 +811,9 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> {
}
}
Err(err) => {
// FIXME: Could we map the tonic StatusCode to a libc errno in a more fine-grained way? Or pass the error message to the backend
info!("tonic error: {err:?}");
return Err(-1);
return Err(libc::EIO);
}
}
Ok(())

69
pgxn/neon/communicator_new.c
View File

@@ -625,10 +625,11 @@ communicator_new_rel_exists(NRelFileInfo rinfo, ForkNumber forkNum)
case NeonIOResult_RelSize:
return result.rel_size != InvalidBlockNumber;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not check existence of rel %u/%u/%u.%u: %s",
RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not check existence of rel %u/%u/%u.%u: %m",
RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for RelSize operation: %d", result.tag);
@@ -796,16 +797,17 @@ retry:
memcpy(buffers[0], bounce_buf_used, BLCKSZ);
return;
case NeonIOResult_Error:
errno = result.error;
if (nblocks > 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read block %u in rel %u/%u/%u.%u: %s",
blockno, RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not read block %u in rel %u/%u/%u.%u: %m",
blockno, RelFileInfoFmt(rinfo), forkNum)));
else
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read %u blocks at %u in rel %u/%u/%u.%u: %s",
nblocks, blockno, RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not read %u blocks at %u in rel %u/%u/%u.%u: %m",
nblocks, blockno, RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for GetPageV operation: %d", result.tag);
@@ -856,10 +858,11 @@ communicator_new_read_at_lsn_uncached(NRelFileInfo rinfo, ForkNumber forkNum, Bl
memcpy(buffer, bounce_buf_used, BLCKSZ);
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read (uncached) block %u in rel %u/%u/%u.%u: %s",
blockno, RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not read (uncached) block %u in rel %u/%u/%u.%u: %m",
blockno, RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for GetPageV operation: %d", result.tag);
@@ -892,10 +895,11 @@ communicator_new_rel_nblocks(NRelFileInfo rinfo, ForkNumber forkNum)
case NeonIOResult_RelSize:
return result.rel_size;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read size of rel %u/%u/%u.%u: %s",
RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not read size of rel %u/%u/%u.%u: %m",
RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for RelSize operation: %d", result.tag);
@@ -924,10 +928,11 @@ communicator_new_dbsize(Oid dbNode)
case NeonIOResult_DbSize:
return (int64) result.db_size;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read database size of database %u: %s",
dbNode, pg_strerror(result.error))));
errmsg("could not read database size of database %u: %m",
dbNode)));
break;
default:
elog(ERROR, "unexpected result for DbSize operation: %d", result.tag);
@@ -978,10 +983,11 @@ communicator_new_read_slru_segment(
nblocks = result.read_slru_segment;
break;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read slru segment, kind=%u, segno=%u: %s",
kind, segno, pg_strerror(result.error))));
errmsg("could not read slru segment, kind=%u, segno=%u: %m",
kind, segno)));
break;
default:
elog(ERROR, "unexpected result for read SLRU operation: %d", result.tag);
@@ -1021,10 +1027,11 @@ communicator_new_write_page(NRelFileInfo rinfo, ForkNumber forkNum, BlockNumber
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write block %u in rel %u/%u/%u.%u: %s",
blockno, RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not write block %u in rel %u/%u/%u.%u: %m",
blockno, RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for WritePage operation: %d", result.tag);
@@ -1061,10 +1068,11 @@ communicator_new_rel_extend(NRelFileInfo rinfo, ForkNumber forkNum, BlockNumber
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not extend to block %u in rel %u/%u/%u.%u: %s",
blockno, RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not extend to block %u in rel %u/%u/%u.%u: %m",
blockno, RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for Extend operation: %d", result.tag);
@@ -1100,10 +1108,11 @@ communicator_new_rel_zeroextend(NRelFileInfo rinfo, ForkNumber forkNum, BlockNum
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not zeroextend to block %u in rel %u/%u/%u.%u: %s",
blockno, RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not zeroextend to block %u in rel %u/%u/%u.%u: %m",
blockno, RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for ZeroExtend operation: %d", result.tag);
@@ -1136,10 +1145,11 @@ communicator_new_rel_create(NRelFileInfo rinfo, ForkNumber forkNum, XLogRecPtr l
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create rel %u/%u/%u.%u: %s",
RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not create rel %u/%u/%u.%u: %m",
RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for Create operation: %d", result.tag);
@@ -1173,10 +1183,11 @@ communicator_new_rel_truncate(NRelFileInfo rinfo, ForkNumber forkNum, BlockNumbe
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not truncate rel %u/%u/%u.%u to %u blocks: %s",
RelFileInfoFmt(rinfo), forkNum, nblocks, pg_strerror(result.error))));
errmsg("could not truncate rel %u/%u/%u.%u to %u blocks: %m",
RelFileInfoFmt(rinfo), forkNum, nblocks)));
break;
default:
elog(ERROR, "unexpected result for Truncate operation: %d", result.tag);
@@ -1209,10 +1220,11 @@ communicator_new_rel_unlink(NRelFileInfo rinfo, ForkNumber forkNum, XLogRecPtr l
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not unlink rel %u/%u/%u.%u: %s",
RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not unlink rel %u/%u/%u.%u: %m",
RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for Unlink operation: %d", result.tag);
@@ -1243,10 +1255,11 @@ communicator_new_update_cached_rel_size(NRelFileInfo rinfo, ForkNumber forkNum,
case NeonIOResult_WriteOK:
return;
case NeonIOResult_Error:
errno = result.error;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not update cached size for rel %u/%u/%u.%u: %s",
RelFileInfoFmt(rinfo), forkNum, pg_strerror(result.error))));
errmsg("could not update cached size for rel %u/%u/%u.%u: %m",
RelFileInfoFmt(rinfo), forkNum)));
break;
default:
elog(ERROR, "unexpected result for UpdateCachedRelSize operation: %d", result.tag);