forked frompostgres/postgres
- Notifications
You must be signed in to change notification settings - Fork6
Commit932b016
committed
Fix cache invalidation bug in recovery_prefetch.
XLogPageRead() can retry internally after a pread() system call hassucceeded, in the case of short reads, and page validation failureswhile in standby mode (see commit0668719). Due to an oversight incommit3f1ce97, these cases could leave stale data in the internalcache of xlogreader.c without marking it invalid. The main defenseagainst stale cached data on failure to read a page was in the errorhandling path of the calling function ReadPageInternal(), but thatwasn't quite enough for errors handled internally by XLogPageRead()'sretry loop if we then exited with XLREAD_WOULDBLOCK.1. ReadPageInternal() now marks the cache invalid before calling the page_read callback, by setting state->readLen to 0. It'll be set to a non-zero value only after a successful read. It'll stay valid as long as the caller requests data in the cached range.2. XLogPageRead() no long performs internal retries while reading ahead. While such retries should work, the general philosophy is that we should give up prefetching if anything unusual happens so we can handle it when recovery catches up, to reduce the complexity of the system. Let's do that here too.3. While here, a new function XLogReaderResetError() improves the separation between xlogrecovery.c and xlogreader.c, where the former previously clobbered the latter's internal error buffer directly. The new function makes this more explicit, and also clears a related flag, without which a standby would needlessly retry in the outer function.Thanks to Noah Misch for tracking down the conditions required for arare build farm failure in src/bin/pg_ctl/t/003_promote.pl, andproviding a reproducer.Back-patch to 15.Reported-by: Noah Misch <noah@leadboat.com>Discussion:https://postgr.es/m/20220807003627.GA4168930%40rfd.leadboat.com1 parentff720a5 commit932b016
File tree
3 files changed
+31
-6
lines changed- src
- backend/access/transam
- include/access
3 files changed
+31
-6
lines changedLines changed: 19 additions & 5 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
987 | 987 |
| |
988 | 988 |
| |
989 | 989 |
| |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
990 | 997 |
| |
991 | 998 |
| |
992 | 999 |
| |
| |||
1067 | 1074 |
| |
1068 | 1075 |
| |
1069 | 1076 |
| |
1070 |
| - | |
1071 |
| - | |
1072 |
| - | |
1073 |
| - | |
1074 |
| - | |
| 1077 | + | |
| 1078 | + | |
1075 | 1079 |
| |
1076 | 1080 |
| |
1077 | 1081 |
| |
| |||
1323 | 1327 |
| |
1324 | 1328 |
| |
1325 | 1329 |
| |
| 1330 | + | |
| 1331 | + | |
| 1332 | + | |
| 1333 | + | |
| 1334 | + | |
| 1335 | + | |
| 1336 | + | |
| 1337 | + | |
| 1338 | + | |
| 1339 | + | |
1326 | 1340 |
| |
1327 | 1341 |
| |
1328 | 1342 |
| |
|
Lines changed: 9 additions & 1 deletion
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
3341 | 3341 |
| |
3342 | 3342 |
| |
3343 | 3343 |
| |
3344 |
| - | |
| 3344 | + | |
3345 | 3345 |
| |
3346 | 3346 |
| |
3347 | 3347 |
| |
3348 | 3348 |
| |
3349 | 3349 |
| |
3350 | 3350 |
| |
| 3351 | + | |
| 3352 | + | |
| 3353 | + | |
| 3354 | + | |
| 3355 | + | |
| 3356 | + | |
| 3357 | + | |
| 3358 | + | |
3351 | 3359 |
| |
3352 | 3360 |
| |
3353 | 3361 |
| |
|
Lines changed: 3 additions & 0 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
373 | 373 |
| |
374 | 374 |
| |
375 | 375 |
| |
| 376 | + | |
| 377 | + | |
| 378 | + | |
376 | 379 |
| |
377 | 380 |
| |
378 | 381 |
| |
|
0 commit comments
Comments
(0)