From d6a26666140ee0330d2b54b3a1f77f6c7a79fd57 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 6 Mar 2023 13:34:03 -0600 Subject: [PATCH] I'm dumb and btree split w/o predicted actually doesn't need two pcaches In B-tree split we turn one rbyd into two by comparing each tag to an id we as a mid-point. I first implemented this by writing both children in parallel, which is efficient, but requires two pcaches for low-level page alignment issues. However we really don't have to write these in parallel. We can just write each child sequentially by making two passes of the original rbyd. --- With this fix, the rewrite of B-tree splitting without predicted rbyd sizes now works. The idea is, instead of predicting the rbyd size to decide whether or not to split, assume we always fit, perform a normal compaction, and if it turns out we don't fit, make a split, writing rm tags as necessary to revert any ids that don't belong in the first child. The neat thing about this is we can use low-level, uncommitting rbyd appends to do all of this in a single commit, avoiding issues with single-prog blocks. This can waste some progs, up to 1/4 of a block during a B-tree split. However, it removes the main need for the rbyd prediction operations, which are complicated, error prone, and concerning. B-tree removes/merges still need an implementation, but this may mean that we can let the on-disk rbyd data-structure be the only source of knowledge about tags, which is great for ensuring consistent behavior. This does mean we don't predict rbyd changes during compaction. Any pending attributes just get appended to the rbyd after compaction, so size-changing operations such as removes can lead to splits that could be avoided. But I think these cases can lead to unnecessary splits anyways depending on when compaction occurs, so I'm not sure it's really an issue. But I can always be wrong about that. --- lfs.c | 75 ++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/lfs.c b/lfs.c index 754888c1..6dc1bed8 100644 --- a/lfs.c +++ b/lfs.c @@ -1888,7 +1888,6 @@ static lfs_ssize_t lfsr_rbyd_bisect(lfs_t *lfs, const lfsr_rbyd_t *rbyd) { if (err && err != LFS_ERR_NOENT) { return err; } - if (err == LFS_ERR_NOENT) { break; } @@ -3215,7 +3214,7 @@ static lfs_ssize_t lfsr_btree_lookup(lfs_t *lfs, static int lfsr_btree_parent(lfs_t *lfs, const lfsr_btree_t *btree, lfs_size_t id, const lfsr_rbyd_t *child, lfsr_rbyd_t *rbyd_, lfs_ssize_t *rid_) { - // inlined? + // inlined? root? if (btree->tag || ( btree->u.trunk.block == child->block && btree->u.trunk.limit == child->off)) { @@ -3319,6 +3318,7 @@ static int lfsr_btree_commit(lfs_t *lfs, lfs_ssize_t pid; int err = lfsr_btree_parent(lfs, btree, id, rbyd, &parent, &pid); if (err && err != LFS_ERR_NOENT) { + assert(!err); return err; } if (err == LFS_ERR_NOENT) { @@ -3334,6 +3334,7 @@ static int lfsr_btree_commit(lfs_t *lfs, err = lfsr_rbyd_commit(lfs, rbyd, attrs); if (err && err != LFS_ERR_RANGE) { // TODO wait should we also move if there is corruption here? + assert(!err); return err; } @@ -3396,9 +3397,8 @@ static int lfsr_btree_commit(lfs_t *lfs, if (err && err != LFS_ERR_NOENT) { return err; } - if (err == LFS_ERR_NOENT) { - goto nosplit; + break; } // TODO this is really wasteful and throws off our predicted @@ -3423,12 +3423,19 @@ static int lfsr_btree_commit(lfs_t *lfs, // keep rbyd < our compaction threshold (1/2) to avoid // degenerate cases - if (rbyd_.off > lfs->cfg->block_size/2){ + if (rbyd_.off > lfs->cfg->block_size/2) { goto split; } } - nosplit:; + // finalize commit with new attrs, it's up to upper + // layers to make sure these always fit + err = lfsr_rbyd_commit(lfs, &rbyd_, attrs); + if (err) { + assert(!err); + return err; + } + // done? if (pid == -1) { *rbyd = rbyd_; @@ -3467,6 +3474,7 @@ static int lfsr_btree_commit(lfs_t *lfs, // find out which id we need to split around lfs_ssize_t bisect = lfsr_rbyd_bisect(lfs, rbyd); if (bisect < 0) { + assert(!err); return bisect; } @@ -3480,9 +3488,33 @@ static int lfsr_btree_commit(lfs_t *lfs, bisect, LFSR_DATA_BUF(NULL, rbyd_.weight-bisect)); if (err) { + assert(!err); return err; } } + + // commit pending attrs, these may need to go into both rbyds, + // upper layers should make sure this can't fail by limiting the + // maximum commit size + // TODO filter-like tag? "from" but from device? + for (const struct lfsr_attr *attr = attrs; attr; attr = attr->next) { + if (attr->id < bisect) { + err = lfsr_rbyd_append(lfs, &rbyd_, + attr->tag, attr->id, + LFSR_DATA_BUF(attr->buffer, attr->size)); + if (err) { + assert(!err); + return err; + } + } + } + + // finalize commit + err = lfsr_rbyd_commit(lfs, &rbyd_, NULL); + if (err) { + assert(!err); + return err; + } // create a sibling and copy remaining ids there, upper layers // should make sure this can't fail by limiting the maximum @@ -3490,6 +3522,7 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_rbyd_t sibling; err = lfsr_rbyd_alloc(lfs, &sibling, rbyd->rev+1); if (err) { + assert(!err); return err; } @@ -3502,8 +3535,12 @@ static int lfsr_btree_commit(lfs_t *lfs, err = lfsr_rbyd_lookup(lfs, rbyd, lfsr_tag_next(tag), id, &tag, &id, &weight, &off, &size); if (err && err != LFS_ERR_NOENT) { + assert(!err); return err; } + if (err == LFS_ERR_NOENT) { + break; + } // TODO this is really wasteful and throws off our predicted // size, can we combine grows into the tag append in the rbyd @@ -3515,14 +3552,16 @@ static int lfsr_btree_commit(lfs_t *lfs, // TODO also this is a weird way to use lfsr_data_t LFSR_DATA_BUF(NULL, weight)); if (err) { + assert(!err); return err; } // append the attr - err = lfsr_rbyd_append(lfs, &rbyd_, + err = lfsr_rbyd_append(lfs, &sibling, tag, id - bisect, LFSR_DATA_DISK(rbyd->block, off, size)); if (err) { + assert(!err); return err; } } @@ -3530,34 +3569,22 @@ static int lfsr_btree_commit(lfs_t *lfs, // commit pending attrs, these may need to go into both rbyds, // upper layers should make sure this can't fail by limiting the // maximum commit size - for (const struct lfsr_attr *attr = attrs; - attr; - attr = attr->next) { - if (attr->id < bisect) { - err = lfsr_rbyd_append(lfs, &rbyd_, - attr->tag, attr->id, - LFSR_DATA_BUF(attr->buffer, attr->size)); - if (err) { - return err; - } - } else { + for (const struct lfsr_attr *attr = attrs; attr; attr = attr->next) { + if (attr->id >= bisect) { err = lfsr_rbyd_append(lfs, &sibling, attr->tag, attr->id-bisect, LFSR_DATA_BUF(attr->buffer, attr->size)); if (err) { + assert(!err); return err; } } } - // finalize both commits - err = lfsr_rbyd_commit(lfs, &rbyd_, NULL); - if (err) { - return err; - } - + // finalize commit err = lfsr_rbyd_commit(lfs, &sibling, NULL); if (err) { + assert(!err); return err; }