Skip to content

Commit

Permalink
tools/schema_loader: read_schema_table_mutation(): close the reader
Browse files Browse the repository at this point in the history
The reader used to read the sstables was not closed. This could
sometimes trigger an abort(), because the reader was destroyed, without
it being closed first.
Why only sometimes? This is due to two factors:
* read_mutation_from_flat_mutation_reader() - the method used to extract
  a mutation from the reader, uses consume(), which does not trigger
  `set_close_is_required()` (#16520). Due to this, the top-level
  combined reader did not complain when destroyed without close.
* The combined reader closes underlying readers who have no more data
  for the current range. If the circumstances are just right, all
  underlying readers are closed, before the combined reader is
  destoyed. Looks like this is what happens for the most time.

This bug was discovered in SCT testing. After fixing #16520, all
invokations of `scylla-sstable`, which use this code would trigger the
abort, without this patch. So no further testing is required.

Fixes: #16519

Closes #16521

(cherry picked from commit da03334)
  • Loading branch information
denesb authored and avikivity committed Dec 31, 2023
1 parent 76c3dda commit 740ba3a
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions tools/schema_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ mutation_opt read_schema_table_mutation(sharded<sstable_manager_service>& sst_ma
readers.emplace_back(sst->make_reader(schema_table_schema, permit, pr, ps));
}
auto reader = make_combined_reader(schema_table_schema, permit, std::move(readers));
auto close_reader = deferred_close(reader);

return read_mutation_from_flat_mutation_reader(reader).get();
}
Expand Down

0 comments on commit 740ba3a

Please sign in to comment.