git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 0/8] reftable: improvements and fixes for compaction
Date: Wed, 31 Jul 2024 16:14:48 +0200	[thread overview]
Message-ID: <cover.1722435214.git.ps@pks.im> (raw)

[-- Attachment #1: Type: text/plain, Size: 2504 bytes --]

Hi,

this patch series addresses a couple of issues with compaction of the
reftable stack. The original intent was to make compaction handle the
case gracefully where a subset of the tables it wants to compact has
been locked already. While working on that I found an issue where
compaction may race with a concurrent writer that modified the
"tables.list" file while compacting it.

In theory, this situation should work alright and has been accounted for
in the design: the compacting process locks "tables.list", then locks
all of the tables it wants to compact, unlocks "tables.list", compacts
the data and re-locks "tables.list" to update the stack. No concurrent
process would thus be able to compact the same tables, and thus we know
that the tables we have just compacted must still exist.

What the code didn't do though is to reload the stack before writing the
updated tables to "tables.list". This could either lead to us dropping
new tables appended to the stack while we were compacting, or it could
lead to us writing references to concurrently-compacted tables which
have been removed meanwhile, leading to data loss.

The fix itself is rather straight-forward. What I'm missing though is a
way to test for this given that the logic only triggers when racing with
another thread. I didn't have any idea how to do this, so if anybody
else has an idea: please, share it! Otherwise, I'm not sure I feel
comfortable with untested medium-complexity code. The alternative would
be to just bail out when we see that the stack has been compacted, which
is less ideal when we have just done a bunch of working compacting large
tables.

Thanks!

Patrick

Patrick Steinhardt (8):
  reftable/stack: refactor function to gather table sizes
  reftable/stack: test compaction with already-locked tables
  reftable/stack: update stats on failed full compaction
  reftable/stack: simplify tracking of table locks
  reftable/stack: do not die when fsyncing lock file files
  reftable/stack: use lock_file when adding table to "tables.list"
  reftable/stack: fix corruption on concurrent compaction
  reftable/stack: handle locked tables during auto-compaction

 reftable/stack.c           | 228 +++++++++++++++++++++++++++++--------
 reftable/stack_test.c      | 104 +++++++++++++++++
 t/t0610-reftable-basics.sh |  21 ++--
 3 files changed, 300 insertions(+), 53 deletions(-)


base-commit: 39bf06adf96da25b87c9aa7d35a32ef3683eb4a4
-- 
2.46.0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

             reply	other threads:[~2024-07-31 14:14 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-31 14:14 Patrick Steinhardt [this message]
2024-07-31 14:14 ` [PATCH 1/8] reftable/stack: refactor function to gather table sizes Patrick Steinhardt
2024-08-02 20:26   ` Junio C Hamano
2024-07-31 14:14 ` [PATCH 2/8] reftable/stack: test compaction with already-locked tables Patrick Steinhardt
2024-08-02 21:05   ` Junio C Hamano
2024-08-05 12:11     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 3/8] reftable/stack: update stats on failed full compaction Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 4/8] reftable/stack: simplify tracking of table locks Patrick Steinhardt
2024-07-31 21:57   ` Justin Tobler
2024-08-02 23:00   ` Junio C Hamano
2024-08-05 12:11     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 5/8] reftable/stack: do not die when fsyncing lock file files Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 6/8] reftable/stack: use lock_file when adding table to "tables.list" Patrick Steinhardt
2024-07-31 23:02   ` Justin Tobler
2024-08-01  8:40     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 7/8] reftable/stack: fix corruption on concurrent compaction Patrick Steinhardt
2024-08-01  1:04   ` Justin Tobler
2024-08-01  8:41     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 8/8] reftable/stack: handle locked tables during auto-compaction Patrick Steinhardt
2024-08-02 23:24   ` Junio C Hamano
2024-08-05 12:11     ` Patrick Steinhardt
2024-08-05 13:07 ` [PATCH v2 0/9] reftable: improvements and fixes for compaction Patrick Steinhardt
2024-08-05 13:07   ` [PATCH v2 1/9] reftable/stack: refactor function to gather table sizes Patrick Steinhardt
2024-08-05 13:07   ` [PATCH v2 2/9] reftable/stack: extract function to setup stack with N tables Patrick Steinhardt
2024-08-05 13:07   ` [PATCH v2 3/9] reftable/stack: test compaction with already-locked tables Patrick Steinhardt
2024-08-08 10:46     ` Karthik Nayak
2024-08-05 13:08   ` [PATCH v2 4/9] reftable/stack: update stats on failed full compaction Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 5/9] reftable/stack: simplify tracking of table locks Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 6/9] reftable/stack: do not die when fsyncing lock file files Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 7/9] reftable/stack: use lock_file when adding table to "tables.list" Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 8/9] reftable/stack: fix corruption on concurrent compaction Patrick Steinhardt
2024-08-08 12:14     ` Karthik Nayak
2024-08-08 13:29       ` Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 9/9] reftable/stack: handle locked tables during auto-compaction Patrick Steinhardt
2024-08-06 18:46     ` Justin Tobler
2024-08-07  5:31       ` Patrick Steinhardt
2024-08-07 19:12         ` Justin Tobler
2024-08-08 12:25     ` Karthik Nayak
2024-08-08 12:26   ` [PATCH v2 0/9] reftable: improvements and fixes for compaction Karthik Nayak
2024-08-08 14:06 ` [PATCH v3 " Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 1/9] reftable/stack: refactor function to gather table sizes Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 2/9] reftable/stack: extract function to setup stack with N tables Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 3/9] reftable/stack: test compaction with already-locked tables Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 4/9] reftable/stack: update stats on failed full compaction Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 5/9] reftable/stack: simplify tracking of table locks Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 6/9] reftable/stack: do not die when fsyncing lock file files Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 7/9] reftable/stack: use lock_file when adding table to "tables.list" Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 8/9] reftable/stack: fix corruption on concurrent compaction Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 9/9] reftable/stack: handle locked tables during auto-compaction Patrick Steinhardt
2024-08-08 16:52   ` [PATCH v3 0/9] reftable: improvements and fixes for compaction Karthik Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1722435214.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).