Don't read space maps [DEBUG USE ONLY] #2909
Conversation
Normally when importing a pool the space maps for all top level vdevs are read from disk. The space maps will be required latter when an allocation is performed and free blocks need to be located. However, if the pool is imported readonly then we are guaranteed that no allocations can occur. In this case the space maps need not be loaded.. A similar argument can be made for the DTLs (dirty time logs). Because a pool import will fail if the space maps cannot be read. The ability to safely ignore them makes it more likely that a damaged pool can be imported readonly to recover its contents. Signed-off-by: Brian Behlendorf behlendorf1@llnl.gov Issue openzfs#2831
Normally when importing a pool the space maps for all top level vdevs are read from disk. The space maps will be required latter when an allocation is performed and free blocks need to be located. However, if the pool is imported readonly then we are guaranteed that no allocations can occur. In this case the space maps need not be loaded.. A similar argument can be made for the DTLs (dirty time logs). Because a pool import will fail if the space maps cannot be read. The ability to safely ignore them makes it more likely that a damaged pool can be imported readonly to recover its contents. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#2831
behlendorf
force-pushed
the
issue-2831
branch
from
1aedf69
to
de771cb
Compare
Normally when importing a pool the space maps for all top level vdevs are read from disk. The space maps will be required latter when an allocation is performed and free blocks need to be located. However, if the pool is imported readonly then we are guaranteed that no allocations can occur. In this case the space maps need not be loaded.. A similar argument can be made for the DTLs (dirty time logs). Because a pool import will fail if the space maps cannot be read. The ability to safely ignore them makes it more likely that a damaged pool can be imported readonly to recover its contents. Signed-off-by: Brian Behlendorf behlendorf1@llnl.gov Issue openzfs#2831
| */ | ||
| if (object != 0) { | ||
| if (object != 0 && spa_writeable(vd->vdev_spa)) { |
There was a problem hiding this comment.
Are we sure that spa_writeable(vd->vdev_spa) returns true in all contexts here? I'm getting the suspicion that #3094 is connected to this or #2784, and this line may explain the pool importing itself sans space map and corrupting all future space maps by writing data during the period it is imported this way without updating the space maps.
|
@behlendorf: #3094 is starting to come together, and signs point strongly to the supposition that pools being imported as writeable always return true for spa_writeable(vd->vdev_spa). Due to the age of this PR, and the fact that ZFS apparently has no capacity to scrub space map errors, the deviations in free space accounting introduced on test hosts, or even in production (looks like im not the only one) were not caught by any of the testers until my pool went south. These deviations got worse over time, and either because i have a heavily used single-vdev pool, or have been running this PR for ~3 months on it, mine just exhibited symptoms first. @everyone: Pending resolution of #3094, i would strongly caution anyone from running this on anything but a throw-away test pool (as in do not allow writable import of important pools on any host running this, there may be some serious dragons down this rabbit hole). |
|
@sempervictus This patch could definitely be related to the space map issues you've reported. It was originally proposed as a last resort to allow an already damaged pool to be importable read-only so the data could be saved. I should have included a clearer warning that it wasn't tested beyond that specific use case. I never meant for it to be used more broadly without significant additional review and testing. This pull request is being closed. It should not be used except as a last resort when attempting pool recovery. |
behlendorf
changed the title
|
Testing confirms - the pool must be in a non writeable state (or the vdev anyway) when the check hits. Thanks for closing this out |
Normally when importing a pool the space maps for all top level
vdevs are read from disk. The space maps will be required latter
when an allocation is performed and free blocks need to be located.
However, if the pool is imported readonly then we are guaranteed
that no allocations can occur. In this case the space maps need
not be loaded.. A similar argument can be made for the DTLs
(dirty time logs).
Because a pool import will fail if the space maps cannot be read.
The ability to safely ignore them makes it more likely that a
damaged pool can be imported readonly to recover its contents.
Signed-off-by: Brian Behlendorf behlendorf1@llnl.gov
Issue #2831