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

Make "read" commands/operations really read-only. #3639

Open
julio-lopez opened this issue Feb 8, 2024 · 0 comments
Open

Make "read" commands/operations really read-only. #3639

julio-lopez opened this issue Feb 8, 2024 · 0 comments
Labels
bug enterprise For anything enterprise related helping reqs from the industry. keep-open resource-utilization ux User eXperience

Comments

@julio-lopez
Copy link
Collaborator

julio-lopez commented Feb 8, 2024

IOW, ensure no writes are performed during read operations such as snapshot list, snapshot restore, repo connect, etc.

The execution of read commands, such as snapshot list among many others, is expected to only read data from the (kopia) repository and not perform any writes or change the repository in any way.

  • Performing read-only operations is safer overall.
  • It allows for use cases where the repository is only accessible in read-only mode, for example, due to credentials that only have read permissions.
  • Performing writes during read commands is often unexpected and may lead to bad user experience, such as taking a long time to complete simple commands such as snapshot list or repo connect.

At the moment, kopia performs a write operations, primarily related to opportunistic cleanup tasks:

The objective is ensure only reads are issued to the storage during these commands.

The short term plan is to address solve the specific issues related to epoch index compaction (#3638) and repository maintenance (#3174).

Long term, we should consider adding the concept of an internal "read-only" session for operations to ensure (potentially at compile time) that no writes are performed in these read-only cases.

@julio-lopez julio-lopez added bug ux User eXperience enterprise For anything enterprise related helping reqs from the industry. keep-open resource-utilization labels Feb 8, 2024
julio-lopez added a commit that referenced this issue Feb 16, 2024
Refactor: move index compaction and cleanup out of refreshAttemptLocked

Introduces an `allowWritesOnLoadHelper` to check whether or not writes
can be performed when loading the indexes. Currently this is only
a function of whether the storage is in read-only mode. In the near
future, an explicit flag will be added to control this behavior.

Fix epoch manager: avoid single-epoch compaction when writes are disallowed.
Functional change: prevents compacting single epochs when writes are
disallowed, that is when using read-only storage. Currently, the epoch
manager will attempt to perform single-epoch compactions for all
eligible epochs, even on read-only storage.

Ref:
- #3224
- #3225
- #3638
- #3639
julio-lopez added a commit that referenced this issue Feb 16, 2024
Conditionally disables epoch index maintenance operations when loading indexes.
This prevents (potentially expensive) cleanup write operations on the index
read path.
The behavior is controlled via the `epoch.Manager.allowCleanupWritesOnIndexLoad`
field, which can be temporarily overridden via an environment variable.
This override mechanism will be removed in the near future.

Refs:

- #3174
- #3224
- #3225
- #3638
- #3639
julio-lopez added a commit to kastenhq/kopia that referenced this issue Mar 12, 2024
julio-lopez added a commit that referenced this issue Mar 12, 2024
Extracted from #3651.

Thanks to @plar and @redgoat650 for the suggestions.

Ref:

- #3603
- #3645
- #3638
- #3639
Lyndon-Li pushed a commit to Lyndon-Li/kopia that referenced this issue Mar 18, 2024
Refactor: move index compaction and cleanup out of refreshAttemptLocked

Introduces an `allowWritesOnLoadHelper` to check whether or not writes
can be performed when loading the indexes. Currently this is only
a function of whether the storage is in read-only mode. In the near
future, an explicit flag will be added to control this behavior.

Fix epoch manager: avoid single-epoch compaction when writes are disallowed.
Functional change: prevents compacting single epochs when writes are
disallowed, that is when using read-only storage. Currently, the epoch
manager will attempt to perform single-epoch compactions for all
eligible epochs, even on read-only storage.

Ref:
- kopia#3224
- kopia#3225
- kopia#3638
- kopia#3639
Lyndon-Li pushed a commit to Lyndon-Li/kopia that referenced this issue Mar 18, 2024
…#3645)

Conditionally disables epoch index maintenance operations when loading indexes.
This prevents (potentially expensive) cleanup write operations on the index
read path.
The behavior is controlled via the `epoch.Manager.allowCleanupWritesOnIndexLoad`
field, which can be temporarily overridden via an environment variable.
This override mechanism will be removed in the near future.

Refs:

- kopia#3174
- kopia#3224
- kopia#3225
- kopia#3638
- kopia#3639
julio-lopez added a commit that referenced this issue Mar 20, 2024
Add:
- epoch.Manager.MaybeCompactSingleEpoch
- getCompactedEpochRange helper
- oldestUncompactedEpoch helper
- TestOldestUncompactedEpoch
- Tests for MaybeCompactSingleEpoch

Ref:
- Subset and dependency of #3651
- Depends on #3735
- Avoid index (epoch) cleanup and compaction during index reads #3638
- Make "read" commands/operations really read-only.  #3639
Lyndon-Li pushed a commit to Lyndon-Li/kopia that referenced this issue Mar 22, 2024
Refactor: move index compaction and cleanup out of refreshAttemptLocked

Introduces an `allowWritesOnLoadHelper` to check whether or not writes
can be performed when loading the indexes. Currently this is only
a function of whether the storage is in read-only mode. In the near
future, an explicit flag will be added to control this behavior.

Fix epoch manager: avoid single-epoch compaction when writes are disallowed.
Functional change: prevents compacting single epochs when writes are
disallowed, that is when using read-only storage. Currently, the epoch
manager will attempt to perform single-epoch compactions for all
eligible epochs, even on read-only storage.

Ref:
- kopia#3224
- kopia#3225
- kopia#3638
- kopia#3639
Lyndon-Li pushed a commit to Lyndon-Li/kopia that referenced this issue Mar 22, 2024
…#3645)

Conditionally disables epoch index maintenance operations when loading indexes.
This prevents (potentially expensive) cleanup write operations on the index
read path.
The behavior is controlled via the `epoch.Manager.allowCleanupWritesOnIndexLoad`
field, which can be temporarily overridden via an environment variable.
This override mechanism will be removed in the near future.

Refs:

- kopia#3174
- kopia#3224
- kopia#3225
- kopia#3638
- kopia#3639
julio-lopez added a commit that referenced this issue Mar 27, 2024
…3651)

Perform index epoch compaction and cleanup during repository maintenance

* refactor: rename maintenance task for deleting superseded indexes
* maintenance task to cleanup epoch markers
* maintenance task to advance write index epoch
* maintenance task to compact epoch ranges
* quick maintenance task to compact a single (index) epoch
* full maintenance task to compact a single (index) epoch

Ref:
- #3638
- #3639

Followup to:
- #3603
- #3645

Related helpers and changes:
- #3721
- #3735
- #3709
- #3728
- #3727
- #3726
Lyndon-Li pushed a commit to Lyndon-Li/kopia that referenced this issue Mar 29, 2024
…opia#3651)

Perform index epoch compaction and cleanup during repository maintenance

* refactor: rename maintenance task for deleting superseded indexes
* maintenance task to cleanup epoch markers
* maintenance task to advance write index epoch
* maintenance task to compact epoch ranges
* quick maintenance task to compact a single (index) epoch
* full maintenance task to compact a single (index) epoch

Ref:
- kopia#3638
- kopia#3639

Followup to:
- kopia#3603
- kopia#3645

Related helpers and changes:
- kopia#3721
- kopia#3735
- kopia#3709
- kopia#3728
- kopia#3727
- kopia#3726
julio-lopez added a commit that referenced this issue Apr 1, 2024
Change default 'allowWritesOnIndexLoad' to false when env var is unset
Add TestNoEpochAdvanceOnIndexRead

Ref:
- Followup to #3645
- Avoid index (epoch) cleanup and compaction during index reads #3638
- Make "read" commands/operations really read-only.  #3639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enterprise For anything enterprise related helping reqs from the industry. keep-open resource-utilization ux User eXperience
Projects
None yet
Development

No branches or pull requests

1 participant