Skip to content

Commit 9b7271c

Browse files
committed
refactor(compaction): extract offline catalog carry-over into a helper
- move the catalog carry-over and update logic out of compaction() into carry_over_and_update_compaction_catalog(), reducing the function's cognitive complexity below the clang-tidy threshold - no behavioral change
1 parent c74e159 commit 9b7271c

1 file changed

Lines changed: 51 additions & 47 deletions

File tree

src/limestone/dblogutil/dblogutil.cpp

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,56 @@ boost::filesystem::path make_backup_dir_next_to(const boost::filesystem::path& t
182182
return make_tmp_dir_next_to(target_dir, ".backup_XXXXXX");
183183
}
184184

185+
// Carry over the existing compaction catalog (and its backup) from from_dir into the
186+
// work directory tmp, then register the freshly produced compacted file. tmp will
187+
// replace from_dir, so carrying over the catalog preserves the high-water marks
188+
// recorded by previous compactions: update_catalog_file keeps max_blob_id
189+
// monotonically non-decreasing, so the freshly computed value (which reflects only
190+
// blobs still referenced by live entries) never lowers it and blob IDs are never
191+
// reused. If from_dir has no catalog (a directory that was never compacted), the empty
192+
// catalog created by setup_initial_logdir(tmp) is used instead.
193+
void carry_over_and_update_compaction_catalog(boost::filesystem::path const& from_dir, boost::filesystem::path const& tmp,
194+
epoch_id_type ld_epoch, blob_id_type max_blob_id) {
195+
auto copy_catalog_file = [&](const std::string& filename) {
196+
boost::filesystem::path src = from_dir / filename;
197+
boost::system::error_code ec;
198+
bool present = boost::filesystem::exists(src, ec);
199+
// A non-existent file is the normal case (boost reports it via ec as ENOENT).
200+
// Any other error (permission, I/O, ...) must not be silently ignored: skipping
201+
// the copy would drop the catalog carry-over and let later processing run on an
202+
// inconsistent state.
203+
if (ec && ec != boost::system::errc::no_such_file_or_directory) {
204+
LOG_AND_THROW_IO_EXCEPTION("failed to check existence of compaction catalog file: " + src.string(), ec);
205+
}
206+
if (present) {
207+
boost::filesystem::copy_file(src, tmp / filename, boost::filesystem::copy_options::overwrite_existing, ec);
208+
if (ec) {
209+
LOG_AND_THROW_IO_EXCEPTION("failed to copy compaction catalog file: " + src.string(), ec);
210+
}
211+
}
212+
};
213+
copy_catalog_file(compaction_catalog::get_catalog_filename());
214+
copy_catalog_file(compaction_catalog::get_catalog_backup_filename());
215+
216+
VLOG_LP(log_info) << "updating compaction catalog in " << tmp;
217+
// tmp always has a catalog here (setup_initial_logdir created an empty one, possibly
218+
// overwritten by the carry-over above), so loading only fails when the carried-over
219+
// catalog and its backup are both unreadable. In that case abort with a clear message
220+
// rather than silently proceeding, which would lose the blob-id high-water mark.
221+
compaction_catalog catalog = [&]() {
222+
try {
223+
return compaction_catalog::from_catalog_file(tmp);
224+
} catch (const limestone_exception& ex) {
225+
LOG_AND_THROW_EXCEPTION(
226+
"the existing compaction catalog in " + from_dir.string() +
227+
" is unreadable and could not be recovered; offline compaction was aborted to"
228+
" avoid losing the blob-id high-water mark (cause: " + ex.what() + ")");
229+
}
230+
}();
231+
compacted_file_info compacted_file{compaction_catalog::get_compacted_filename(), 1};
232+
catalog.update_catalog_file(ld_epoch, max_blob_id, {compacted_file}, {});
233+
}
234+
185235
void compaction(dblog_scan &ds, std::optional<epoch_id_type> epoch) {
186236
epoch_id_type ld_epoch{};
187237
if (epoch.has_value()) {
@@ -252,53 +302,7 @@ void compaction(dblog_scan &ds, std::optional<epoch_id_type> epoch) {
252302
// Without this, a subsequent startup treats the directory as if no compaction
253303
// had been performed, and remove entries are dropped from the snapshot,
254304
// resurrecting deleted records (see tsurugi-issues #1498).
255-
//
256-
// tmp will replace from_dir, so carry over the existing catalog (and its backup)
257-
// before updating it, exactly like online compaction updates the live catalog.
258-
// This preserves the high-water marks recorded by previous compactions: in
259-
// particular update_catalog_file keeps max_blob_id monotonically non-decreasing,
260-
// so the freshly computed max_blob_id (which reflects only blobs still referenced
261-
// by live entries) never lowers it and blob IDs are never reused. If from_dir has
262-
// no catalog (a directory that was never compacted), the empty catalog created by
263-
// setup_initial_logdir(tmp) is used instead.
264-
auto copy_catalog_file = [&](const std::string& filename) {
265-
boost::filesystem::path src = from_dir / filename;
266-
boost::system::error_code ec;
267-
bool present = boost::filesystem::exists(src, ec);
268-
// A non-existent file is the normal case (boost reports it via ec as ENOENT).
269-
// Any other error (permission, I/O, ...) must not be silently ignored: skipping
270-
// the copy would drop the catalog carry-over and let later processing run on an
271-
// inconsistent state.
272-
if (ec && ec != boost::system::errc::no_such_file_or_directory) {
273-
LOG_AND_THROW_IO_EXCEPTION("failed to check existence of compaction catalog file: " + src.string(), ec);
274-
}
275-
if (present) {
276-
boost::filesystem::copy_file(src, tmp / filename, boost::filesystem::copy_options::overwrite_existing, ec);
277-
if (ec) {
278-
LOG_AND_THROW_IO_EXCEPTION("failed to copy compaction catalog file: " + src.string(), ec);
279-
}
280-
}
281-
};
282-
copy_catalog_file(compaction_catalog::get_catalog_filename());
283-
copy_catalog_file(compaction_catalog::get_catalog_backup_filename());
284-
285-
VLOG_LP(log_info) << "updating compaction catalog in " << tmp;
286-
// tmp always has a catalog here (setup_initial_logdir created an empty one, possibly
287-
// overwritten by the carry-over above), so loading only fails when the carried-over
288-
// catalog and its backup are both unreadable. In that case abort with a clear message
289-
// rather than silently proceeding, which would lose the blob-id high-water mark.
290-
compaction_catalog catalog = [&]() {
291-
try {
292-
return compaction_catalog::from_catalog_file(tmp);
293-
} catch (const limestone_exception& ex) {
294-
LOG_AND_THROW_EXCEPTION(
295-
"the existing compaction catalog in " + from_dir.string() +
296-
" is unreadable and could not be recovered; offline compaction was aborted to"
297-
" avoid losing the blob-id high-water mark (cause: " + ex.what() + ")");
298-
}
299-
}();
300-
compacted_file_info compacted_file{compaction_catalog::get_compacted_filename(), 1};
301-
catalog.update_catalog_file(ld_epoch, max_blob_id, {compacted_file}, {});
305+
carry_over_and_update_compaction_catalog(from_dir, tmp, ld_epoch, max_blob_id);
302306

303307
if (FLAGS_dry_run) {
304308
std::cout << "compaction will be successfully completed (dry-run mode)" << std::endl;

0 commit comments

Comments
 (0)