Project

General

Profile

Bug #213

non-idempotent transactions (clone) under ext3 may not replay correct result

Added by Sage Weil about 8 years ago. Updated almost 7 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
OSD
Target version:
Start date:
06/19/2010
Due date:
% Done:

0%

Source:
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:

Description

The writeahead journaling will restore the store to a known state regardless of which operations have committed, but only if the transactions are idempotent. i.e. can be repeated and still end up at a known result. This is true of all the normal operations (remove, write, truncate, setxattr, etc., but not for clone and clone_range. E.g.,

clone foo_head -> foo_2
write foo_head

will put the old head content in _2, but if it is replayed _2 will contain the new _head content. Meh!

How to fix? We could mark which transaction are non-idempotent, and sync the store before applying them.. that's expensive, but possibly the price you pay for not using btrfs! :)


Related issues

Related to Ceph - Bug #2098: xfs/ext4 non-idempotent transaction Resolved 02/23/2012

Associated revisions

Revision 69cd3625 (diff)
Added by Sage Weil almost 7 years ago

filestore: sync after non-idempotent operations

This is a big hammer to fix journal replay on non-btrfs fs backends (extN,
xfs, whatever). The problem is that it is not safe to replay some journal
operations more than once, notably things like CLONE whose source data
may be changed by subsequent operations.

The simple fix is to initiate a full commit after any non-idempotent
operations prior to any subsequent operation within the same Sequencer.
This is done by calling trigger_commit() in _do_transactions(), which means
any potentially dependent operation that follows will get blocked because
a commit is about to start.

I made trigger_commit() a bit more robust to callers who are not holding
an open_ops ref to also succeeding if the given op_seq is already
committing. For the current caller, that can't happen.

There are probably better performing solutions, but this one is at least
correct.

Fixes: #213
Signed-off-by: Sage Weil <>

History

#1 Updated by Sage Weil about 8 years ago

  • Target version changed from v0.21 to v0.22

#2 Updated by Sage Weil almost 8 years ago

  • Target version changed from v0.22 to v0.23

#3 Updated by Sage Weil almost 8 years ago

  • Target version changed from v0.23 to v0.24

#4 Updated by Sage Weil almost 8 years ago

  • Target version deleted (v0.24)

#5 Updated by Sage Weil almost 7 years ago

  • Priority changed from High to Normal

#6 Updated by Sage Weil almost 7 years ago

  • translation missing: en.field_position deleted (614)
  • translation missing: en.field_position set to 1

#7 Updated by Anonymous almost 7 years ago

Isn't the idempotency in that case "clone foo_head -> foo_2 IFF foo_2 does not exist" ?

#8 Updated by Sage Weil almost 7 years ago

Tommi Virtanen wrote:

Isn't the idempotency in that case "clone foo_head -> foo_2 IFF foo_2 does not exist" ?

That's almost enough for clone() (if we add O_EXCL and whitelist EEXIST for non-btrfs). It wouldn't catch something like

1 clone A->B
2 modify A
...
3 delete B
4 &lt;crash&gt;
&lt;replay from 1&gt;

That trick also wouldn't work for clone_range(), which doesn't create a file.

It may be that we need to make transactions idempotent at a higher level, but it'd be dependent on what you clone to, and whether it is ever modified/removed... it'd be dependent on the particular, though, and hard to analyze/verify.

#9 Updated by Sage Weil almost 7 years ago

FWIW even if we know what not to replay, we could still be screwed with ext4 (which does not commit everything in order):

clone A->B
modify A
&lt;fs commits A (before B)&gt;
&lt;crash&gt;

On replay, we don't actually have the old A to clone to B. :(

#10 Updated by Sage Weil almost 7 years ago

I think the simplest solution would be:

- for all operations, set an xattr with the last op_seq to write to that file.
- for any operation that is potentially non-idempotent, fsync(2) after doing it.
- on replay, verify the xattr isn't == or newer to avoid re-doing the operation.

Those operations would be:

- create collection
- clone
- clone range

We need to set the attr on all operations to avoid something like

1 truncate B
2 clone A->B
3 modify A
&lt;crash&gt;
&lt;replay 1&gt;
&lt;skip 2 due to xattr&gt;
&lt;replay 3&gt;

#11 Updated by Sage Weil almost 7 years ago

  • translation missing: en.field_story_points deleted (0)
  • translation missing: en.field_position deleted (6)
  • translation missing: en.field_position set to 6

#12 Updated by Sage Weil almost 7 years ago

  • translation missing: en.field_position deleted (9)
  • translation missing: en.field_position set to 7

#13 Updated by Sage Weil almost 7 years ago

  • translation missing: en.field_story_points set to 5
  • translation missing: en.field_position deleted (7)
  • translation missing: en.field_position set to 7

#14 Updated by Sage Weil almost 7 years ago

  • translation missing: en.field_position deleted (7)
  • translation missing: en.field_position set to 4

#15 Updated by Sage Weil almost 7 years ago

  • Target version set to v0.39
  • translation missing: en.field_position deleted (1)
  • translation missing: en.field_position set to 971

#16 Updated by Sage Weil almost 7 years ago

Update: the current first pass plan is to initiate a FileStore sync after any non-idempotent operation. This updates commit_op_seq on disk and ensures that it won't be replayed.

It's also heavyweight as it calls sync(2). So it's a big hammer, but at least it's correct.

We can set a non-idempotent bool in the do_transaction method on CLONE or anything similar and then do the commit at the end (before any other operations occur under the
current OpSequencer).

#17 Updated by Sage Weil almost 7 years ago

  • Status changed from New to Testing
  • Assignee set to Sage Weil

#18 Updated by Sage Weil almost 7 years ago

  • Status changed from Testing to Resolved

Also available in: Atom PDF