Major - P3
I apologize for the shotgun tag on this ticket, but I'm having problems figuring out what the plan is for corrupted files in WiredTiger.
We have the WT_CONNECTION.WT_CONN_DATA_CORRUPTION flag, and what we mostly do is:
- set the WT_SESSION. WT_SESSION_QUIET_CORRUPT_FILE flag,
- make block manager calls,
- check the WT_CONNECTION.WT_CONN_DATA_CORRUPTION flag if the block manager call returns an error.
This seems fragile to me, because we require in the RTS code that ret == WT_ERROR before we check that corrupted flag, which leads to https://github.com/wiredtiger/wiredtiger/blob/develop/src/block/block_open.c#L360-L370, added in
I don't see any reason to think we'll be able to keep the error handling consistent across future iterations of the code, this seems like a bug waiting to happen?
The reason I started looking at this is that RTS doesn't clear the WT_CONNECTION.WT_CONN_DATA_CORRUPTION flag, so any failure to roll a file forward due to corruption is going to leave that flag set. I don't see how that can be correct – I don't see how an application can subsequently get control of the database to run salvage.
My naive view of this is RTS would reasonably detect corruption in a single file and report it, but wouldn't crash the system.
Of course, I may be totally wrong on all of this, in which case I'd really appreciate someone explaining this to me!
If I'm correct, then I think we should:
- Add a session flag that gets set in the case of block-manager corruption, and query that flag if a block manager call returns an error. Since the block-manager will panic if WT_SESSION_QUIET_CORRUPT_FILE isn't set, there aren't that many paths that will need review.
- At a higher level, we can set the WT_CONN_DATA_CORRUPTION flag of course, but the block manager shouldn't be doing that work.
- Specifically, RTS would report the corruption and not set the WT_CONN_DATA_CORRUPTION flag.
This helps us move toward gracefully handling the corruption of a specific collection in the future, I think, although it doesn't entirely solve that issue.
Everybody tagged here has modified the handling of the WT_CONNECTION.WT_CONN_DATA_CORRUPTION flag in some chunk of code (well, except Alex, I'm copying him for an architectural view). If you don't want to be part of this conversation, please just flag that in the ticket: ravi.giri, donald.anderson, sue.loverso, alex.cameron, haribabu.kommi, alexander.gorrod.