- Notifications
You must be signed in to change notification settings - Fork4.9k
Commitccfbd92
committed
Replace existing durable_rename_excl() calls with durable_rename()
durable_rename_excl() attempts to avoid overwriting any existing filesby using link() and unlink(), falling back to rename() on some platforms(e.g., Windows where link() followed by unlink() is not concurrent-safe,see909b449). Most callers of durable_rename_excl() use it just in casethere is an existing file, but it happens that for all of them we neverexpect a target file to exist (WAL segment recycling, creation oftimeline history file and basic_archive).basic_archive used durable_rename_excl() to avoid overwriting an archiveconcurrently created by another server. Now, there is a stat() call toavoid overwriting an existing archive a couple of lines above, so notethat this change opens a small TOCTOU window in this module between thestat() call and durable_rename().Furthermore, as mentioned in the top comment of durable_rename_excl(),this routine can result in multiple hard links to the same file and datacorruption, with two or more links to the same file in pg_wal/ if acrash happens before the unlink() call during WAL recycling.Specifically, this would produce links to the same file for the currentWAL file and the next one because the half-recycled WAL file wasre-recycled during crash recovery of a follow-up cluster restart.This change replaces all calls to durable_rename_excl() withdurable_rename(). This removes the protection against accidentallyoverwriting an existing file, but some platforms are already livingwithout it, and all those code paths never expect an existing file (acouple of assertions are added to check after that, in case).This is a bug fix, but knowing the unlikeliness of the problem involvingone of more crashes at an exceptionally bad moment, no backpatch isdone. This could be revisited in the future.Author: Nathan BossartReviewed-by: Robert Haas, Kyotaro Horiguchi, Michael PaquierDiscussion:https://postgr.es/m/20220407182954.GA1231544@nathanxps131 parent755df30 commitccfbd92
File tree
3 files changed
+13
-20
lines changed- contrib/basic_archive
- src/backend/access/transam
3 files changed
+13
-20
lines changedLines changed: 3 additions & 2 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
281 | 281 |
| |
282 | 282 |
| |
283 | 283 |
| |
284 |
| - | |
| 284 | + | |
| 285 | + | |
285 | 286 |
| |
286 |
| - | |
| 287 | + | |
287 | 288 |
| |
288 | 289 |
| |
289 | 290 |
| |
|
Lines changed: 6 additions & 12 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
439 | 439 |
| |
440 | 440 |
| |
441 | 441 |
| |
| 442 | + | |
442 | 443 |
| |
443 | 444 |
| |
444 |
| - | |
445 |
| - | |
446 |
| - | |
447 |
| - | |
448 |
| - | |
449 |
| - | |
| 445 | + | |
| 446 | + | |
450 | 447 |
| |
451 | 448 |
| |
452 | 449 |
| |
| |||
517 | 514 |
| |
518 | 515 |
| |
519 | 516 |
| |
| 517 | + | |
520 | 518 |
| |
521 | 519 |
| |
522 |
| - | |
523 |
| - | |
524 |
| - | |
525 |
| - | |
526 |
| - | |
527 |
| - | |
| 520 | + | |
| 521 | + | |
528 | 522 |
| |
529 | 523 |
| |
530 | 524 |
| |
|
Lines changed: 4 additions & 6 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
3323 | 3323 |
| |
3324 | 3324 |
| |
3325 | 3325 |
| |
3326 |
| - | |
3327 |
| - | |
3328 |
| - | |
3329 |
| - | |
3330 |
| - | |
| 3326 | + | |
| 3327 | + | |
| 3328 | + | |
3331 | 3329 |
| |
3332 | 3330 |
| |
3333 |
| - | |
| 3331 | + | |
3334 | 3332 |
| |
3335 | 3333 |
| |
3336 | 3334 |
| |
|
0 commit comments
Comments
(0)