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

Refactor round storage methods to return actions #303

Open
kellpossible opened this issue Jul 13, 2021 · 1 comment · May be fixed by #367
Open

Refactor round storage methods to return actions #303

kellpossible opened this issue Jul 13, 2021 · 1 comment · May be fixed by #367
Labels
refactor Changes which affect the maintainability of the code

Comments

@kellpossible
Copy link

kellpossible commented Jul 13, 2021

The new Round::reset() introduced in #288 returns storage actions rather than directly mutating the storage. This easier to test and reason about. There are other methods in Round which could be refactored to work the same way, and have more unit tests written for them:

  • new()
  • remove_contributor_unsafe (introduced during Round restart #288 to combine removed_locks_unsafe and remove_chunk_contributions_unsafe)
  • remove_locks_unsafe
  • remove_chunk_contributions_unsafe
  • try_lock_chunk

The following methods depend on an immutable reference to Storage, perhaps they could be decoupled:

  • current_contribution_locator
  • next_contribution_locator
  • next_contribution_file_signature_locator
@kellpossible kellpossible added the refactor Changes which affect the maintainability of the code label Jul 13, 2021
kellpossible added a commit that referenced this issue Aug 30, 2021
+ remove_contributor_unsafe
+ remove_locks_unsafe
+ remove_chunk_contributions_unsafe
kellpossible added a commit that referenced this issue Aug 30, 2021
@kellpossible kellpossible linked a pull request Aug 30, 2021 that will close this issue
@kellpossible kellpossible linked a pull request Aug 30, 2021 that will close this issue
@kellpossible
Copy link
Author

kellpossible commented Sep 2, 2021

Something that came up during discussions with @ibaryshnikov was the need for transactional changes to the files on disk (#372 ), the ability to roll back if there is a problem during one of the changes in a transaction. There are probably many ways to achieve this that can be explored, this could be orthogonal to the changes being made for this issue, however one idea I did have is that a group of actions could be treated as a transaction, it would then be fairly simple to create a new Disk::process_actions function which provides the ability to reverse changes (by storing and applying opposite actions) when encountering an error (rather than simply aborting as it does now, leaving the file system in an incorrect state).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Changes which affect the maintainability of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant