Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Volume atomic multi-slot appends. #149

Merged
merged 19 commits into from
May 24, 2024

Conversation

tonyastolfi
Copy link
Collaborator

@tonyastolfi tonyastolfi commented May 13, 2024

  • Adds llfs::Volume::MultiAppend, for atomic appending of multiple slots
  • Fixes a bug in Volume's append job, where an error status returned from preparing the job could (incorrectly) fail a check (generating a panic)
  • Adds support for block-device level simulation of LogDevice in StorageSimulation, based on IoRingLogDevice
  • Fixes bug where moving an IoRing object after using it to construct an IoRing::File would result in a dangling pointer
  • Minor enhancements to various APIs:
    • variable-length integer pack/unpack now accept ConstBuffer/MutableBuffer
    • DataReader::read_token added
    • separate halt() and join() member functions for LogDevice, instead of the combined close() function (now deprecated)
    • relax the restriction that LogDevice::prepare may only be called once per commit (now you can call it multiple times to progressively increase the size of the prepared buffer, provided there is space in the log)
  • Consolidated various unit test configuration parameters in llfs::testing::TestConfig (llfs/testing/test_config.hpp)
  • Renamed llfs::PageArena::close to halt to be more consistent with other parts of the code
  • Added new diagnostics to StorageSimulation when a possible deadlock is detected (main test function has exited and there is no runnable work, but the work count on the simulation execution context is non-zero)
  • Fixed a bug in static/runtime configuration of (ioring) log devices where it was possible to create more flush ops than the number of blocks in total, resulting in race conditions where multiple flush ops would write data to the same location(s)

@tonyastolfi tonyastolfi self-assigned this May 13, 2024
Copy link
Collaborator

@gabrielbornstein gabrielbornstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just got started at the end of day today. I'll have more comments, but here's my first couple.

src/llfs/ioring_file.hpp Show resolved Hide resolved
src/llfs/ioring_log_config.hpp Outdated Show resolved Hide resolved
src/llfs/ioring_log_device.hpp Outdated Show resolved Hide resolved
src/llfs/ioring_log_device.hpp Show resolved Hide resolved
src/llfs/ioring_log_driver.hpp Outdated Show resolved Hide resolved
src/llfs/ioring_log_driver.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@gabrielbornstein gabrielbornstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of more comments I made yesterday and forgot to publish.

src/llfs/ioring_log_driver.ipp Outdated Show resolved Hide resolved
src/llfs/ioring_log_driver.ipp Show resolved Hide resolved
src/llfs/ioring_log_initializer.cpp Show resolved Hide resolved
src/llfs/log_storage_driver_context.hpp Show resolved Hide resolved
src/llfs/memory_log_device.hpp Show resolved Hide resolved
src/llfs/simulated_log_device_storage.hpp Show resolved Hide resolved
src/llfs/simulated_log_device_storage.cpp Show resolved Hide resolved
src/llfs/simulated_log_device_storage.cpp Show resolved Hide resolved
src/llfs/simulated_log_device_storage.cpp Outdated Show resolved Hide resolved
src/llfs/simulated_log_device_storage.cpp Outdated Show resolved Hide resolved
src/llfs/simulated_log_device_storage.cpp Show resolved Hide resolved
src/llfs/simulated_log_device_storage.cpp Show resolved Hide resolved
src/llfs/slot_writer.hpp Show resolved Hide resolved
src/llfs/slot_writer.hpp Show resolved Hide resolved
src/llfs/slot_writer.hpp Show resolved Hide resolved
Copy link
Collaborator

@gabrielbornstein gabrielbornstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with feedback! Let me know your questions.

src/llfs/slot_writer.test.cpp Show resolved Hide resolved
src/llfs/storage_simulation.cpp Show resolved Hide resolved
src/llfs/testing/test_config.cpp Outdated Show resolved Hide resolved
src/llfs/volume.cpp Show resolved Hide resolved
src/llfs/volume_multi_append.test.cpp Show resolved Hide resolved
src/llfs/volume_multi_append.test.cpp Show resolved Hide resolved
@tonyastolfi tonyastolfi merged commit 23ecf73 into mathworks:main May 24, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants