Clear up the confusing "precommit(ed)" terminology

PATCH New Zdenek Kabelac 2 participants 2 messages Sep 1, 2011 1 message not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Zdenek Kabelac

author

2011-09-01 14:23 UTC #1 lore raw

PATCH 1/1 Clear up the confusing "precommit(ed)" terminology

Dne 31.8.2011 20:57, Petr Rockai napsal(a):

Quoted 50 lines

Hi,
we have discussed the current use of "precommit" and "precommitted" terminology as related to LVM metadata with Alasdair, and arrived at the following conclusion:
The *state* of the metadata, what was most of the time called "precommitted" (and sometimes "precommit") is changed to "tentative".
The *act* of writing the "precommitted" (i.e. tentative) metadata, which has been called "precommit" is now more explicitly part of vg_write (the *action* of precommitting only existed as part of metadata_area_ops and was called vg_precommit). To reflect the two-phase nature of vg_write with multiple MDAs, I have renamed vg_write in metadata_area_ops to vg_write_phase1 and vg_precommit to vg_write_phase2. Other suggestions have been made, for now I picked phase1/phase2 because it is the easiest to track down and rename to any other combination of names.
The current (proposed) terminology therefore is:
- the metadata after vg_write, but before vg_commit is "tentative"
- the metadata after vg_write *and* vg_commit is "committed"
There is no user-visible state between not-yet-written and tentative (even though vg_write is internally a 2-phase operation, this is not exposed in the metadata layer API).
I am attaching 2 diffs, the first is the changes I did manually, the second is these changes with
find -name \*.\[hc\] | xargs sed -i.orig -e 's,precommitted,tentative,g;s,PRECOMMIT\(TED\)\?,TENTATIVE,g;s,pre-committed,tentative,g;'
on top (you can use interdiff to see just the diff produced by sed).
Yours,
Petr
tentative_manual.diff
ntext *fidtc = (struct text_fid_context *) fid->private;
(quoted diff) +1 -1
@@ -575,7 +575,7 @@ static int _vg_write_raw(struct format_i
 	int r = 0;
        uint64_t new_wrap = 0, old_wrap = 0, new_end;
 	int found = 0;
-	int noprecommit = 0;
+	int notentative = 0;
 
 	/* Ignore any mda on a PV outside the VG. vgsplit relies on this */
 	dm_list_iterate_items(pvl, &vg->pvs) {

Review static int _vg_write_raw(struct format_i · line 599

Quoted 6 lines

(quoted diff) +1 -1

@@ -595,7 +595,7 @@ static int _vg_write_raw(struct format_i
 		goto_out;
 
 	rlocn = _find_vg_rlocn(&mdac->area, mdah,
-			vg->old_name ? vg->old_name : vg->name, &noprecommit);
+			vg->old_name ? vg->old_name : vg->name, &notentative);
 	mdac->rlocn.offset = _next_rlocn_offset(rlocn, mdah);

I'm certainly not an English linguist expert here - but "notentative" looks even more weird.

For my understanding precommit made more sense then tentative - I'm not objecting the idea of cleaning it - but this term makes it even more confusing. If we are going to use 2-phase commit - I'd use term phase1/phase2 here.

Zdenek

Petr Rockai

2011-09-01 15:54 UTC #2 lore raw

PATCH 1/1 Clear up the confusing "precommit(ed)" terminology

Quoted 2 lines · Zdenek Kabelac

I'm certainly not an English linguist expert here - but "notentative" looks even more weird.

Well, notentative is a sed artefact, and can (and should) be cleaned up. I did not because it's not even clear what it was supposed to mean even before (because the same thing is apparenty called precommit and noprecommit in different places?).

Quoted 4 lines

For my understanding precommit made more sense then tentative - I'm not objecting the idea of cleaning it - but this term makes it even more confusing. If we are going to use 2-phase commit - I'd use term phase1/phase2 here.

Only that this is not a 2-phase commit. This is a write and a commit. The "precommit" does not commit anything, and is *not* part of the commit per se. Think in terms of a transaction:

vg = vg_open(...) // open a transaction, acquire lock

// do stuff

vg_write(...) // write an intermediate state

// do more stuff

vg_write(...) // realize you need to actually write a different version

if (everything_ok)
vg_commit(vg); // commit the transaction
else
vg_revert(vg); // abort the transaction

If vg_write was phase1 of a commit, it would make no sense to do anything between phase1 (vg_write) and phase2 (vg_commit). Even the existing code (which uses the concept of transaction only very cautiously) already needs to do things between the write and the commit.

On the other hand, dragging in two-phase commit terminology (which normally describes a distributed database transaction protocol; we do no such thing) would add even more confusion...

Yours,
Petr

Signature · 1 line

id' Ash = Ash; id' Dust = Dust; id' _ = undefined