On Mon, Dec 08, 2025 at 12:48:52PM +0530, Nitin Jadhav wrote: >> An update of src/test/recovery/meson.build to add the new test is >> also required. > > I will work on improving the test accordingly and include the changes > in the next version.
Cool, thanks. > The main reason I chose PANIC is that when the ReadCheckpointRecord() > fails, we already use PANIC for the error message ‘could not locate a > valid checkpoint record…’ in the no_backup_label case, whereas the > similar flow with backup_label uses FATAL. I am not entirely sure why > this difference exists. I looked into it but couldn’t find much. If we > decide to change this patch to use FATAL for ‘could not find redo > location…’, should we also consider changing the existing PANIC to > FATAL for consistency? Using PANIC is an inherited historical artifact that has been introduced around 4d14fe0048cf with the introduction of WAL. There was nothing like archiving or even base backup back then. Switching the existing surrounding one to also use a FATAL is something that seems worth considering to me for the checkpoint record, at least based on the pattern that there could be a driver error even if there is no backup_label file (aka for example the case of FS-levelsnapshots with one partition used for the data folder, no?). This offers bonus points in the shape of more tests like the one you have sent upthread. It's not something that I would backpatch as it is a behavior change, but I'm open to seeing that as an improvement in usability for future releases: PANIC is for cases that should never happen for internal states, due to an internal logic error, or an OS going crazy. Here we have a should-no-happen case triggered by a user, and a FATAL still provides the same information about what's wrong. Let's make such changes separate patches, of course, depending on what we find on the way. > Yes. As noted in my initial email, this patch is focused solely on > fixing the crash issue. It does not address error handling in > StartupXLOG(), CreateCheckPoint(), or involve any broader code > cleanup. That sounds fine to me. -- Michael
signature.asc
Description: PGP signature
