Skip to contentSkip to conversation

DDF: regression caused by 273989b9 / ce45c819

DISCUSSION Martin Wilck 2 participants 2 messages Aug 6, 2013 – Aug 7, 2013 MD RAID

lore
Share & tools
lore
#0 lore raw

Hi Neil,

these patches break the unit test 10ddf-geometry. I saw the regression with both patches applied. The problem occurs when subarrays are deleted. With these patches in place, sync_metadata() will not overwrite deleted conf records on disk, but lseek() over them instead. When the meta data is read back, this will cause errors.

I would like to ask you to play safe here and revert these patches. It might be possible to fix the --kill-subarray problem, but there are other possible scenarios where the number of valid conf records on a disk decreases - I don't think we have a reliable way to check whether it is safe to skip over empty entries. We must also be prepared for other DDF implementations to read our meta data, so we must refrain from putting any writing anything that might be confusing.

Regards
Martin

#1 lore raw
Quoted 16 lines · On Tue, 06 Aug 2013 20:14:35 +0200 Martin Wilck <[email protected]> wrote:

Hi Neil,

these patches break the unit test 10ddf-geometry. I saw the regression with both patches applied. The problem occurs when subarrays are deleted. With these patches in place, sync_metadata() will not overwrite deleted conf records on disk, but lseek() over them instead. When the meta data is read back, this will cause errors.

I would like to ask you to play safe here and revert these patches. It might be possible to fix the --kill-subarray problem, but there are other possible scenarios where the number of valid conf records on a disk decreases - I don't think we have a reliable way to check whether it is safe to skip over empty entries. We must also be prepared for other DDF implementations to read our meta data, so we must refrain from putting any writing anything that might be confusing.

Quoted 2 lines

Regards
Martin

Yes, you are right. That wasn't such a good idea, thanks.

I've changed it to allocate a large buffer (which is done for 'read' - and we use the same buffer), fill that in and write. That is a lot faster than lots of individual writes, due to the fact that the fd is O_DIRECT.

Thanks,
NeilBrown