os/bluestore: fix no metadata update on truncate+fsync #45883
os/bluestore: fix no metadata update on truncate+fsync #45883
Conversation
|
Igor, It is not being set on BlueFS->preallocate() BlueFS->open_for_write() File->append() or any other operation. Is the semantics that a file become dirty only after flush()? |
|
jenkins test make check |
f528825 to
e3777ba
Compare
IIUC the original meaning of the flag was to note a file that has got some content flushed to disk but not yet synced (i.e. metadata isn't in the log yet). |
|
jenkins test make check |
|
@aclamk - ping |
| h->file->fnode.size = offset; | ||
| h->file->is_dirty = true; | ||
| vselector->add_usage(h->file->vselector_hint, h->file->fnode.size); | ||
| log.t.op_file_update_inc(h->file->fnode); |
There was a problem hiding this comment.
It unfortunate we have now both direct update of log.t and signaling is_dirty.
How about:
{
std::lock_guard ll(log.lock);
vselector->sub_usage(h->file->vselector_hint, h->file->fnode.size);
h->file->fnode.size = offset;
vselector->add_usage(h->file->vselector_hint, h->file->fnode.size);
log.t.op_file_update_inc(h->file->fnode);
}
_flush_and_sync_log_LD(0);
This way we would be in-sync right after truncate(), without need to fsync().
There was a problem hiding this comment.
Forget it.
I just realized, that sequence truncate(h); fsync(h); should give us proper results even if we ship sync_metadata() or umount().
|
jenkins test make check arm64 |
|
@ifed I checked that the same problem occurs in pacific. Do we want to backport fix to octopus/pacifc? |
Apparently "no" for Octopus and "may be" for Pacific. Honestly I haven't seen that issue under regular usage - I only faced it when developing that new BlueWAL functionality... |
|
jenkins test signing |
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>
Fixes: https://tracker.ceph.com/issues/55307 Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>
e3777ba to
73a7e92
Compare
|
jenkins test make check |
1 similar comment
|
jenkins test make check |
|
jenkins test make check |
|
Teuthology Test Result Unrelated Failures
|
|
jenkins test this please |
This effectively enables having 4K allocation units for BlueFS. But it doesn't turn it on by default for the sake of performance. Using main device which lacks enough free large continuous extents might do the trick though. Signed-off-by: Igor Fedotov <igor.fedotov@croit.io> (cherry picked from commit 001b08d) Conflicts: src/os/bluestore/BlueFS.cc (trivial, no ceph#39871) src/os/bluestore/BlueStore.cc (trivial, no commits for zoned support) src/test/objectstore/test_bluefs.cc (trivial, no ceph#45883)
Fixes: https://tracker.ceph.com/issues/55307
Although met this during new WAL development only IMO this is potentially dangerous for NCB stuff as well.
Signed-off-by: Igor Fedotov igor.fedotov@croit.io
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows