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

Caching Rook addons resources #1353

Merged
merged 1 commit into from
May 2, 2024
Merged

Caching Rook addons resources #1353

merged 1 commit into from
May 2, 2024

Conversation

abhijeet219
Copy link
Member

@abhijeet219 abhijeet219 commented Apr 22, 2024

Currently the addons fetch kustomization resources whenever they are started.
This change will start using cache for those resources, so starting an addon can directly use the cached resources.

Changes:

  • drenv fetch can be used to fetch resources anytime.
  • Starting an addon will first try to fetch resources, then apply the fetched resources. If there is no change, fetch won't do anything, so takes very less time.

Example demonstrating how the cache works:

Fetching of resources for envs/regional-dr.yaml:

$ drenv fetch envs/regional-dr.yaml -v
2024-05-02 01:47:48,116 INFO    [rdr] Fetching
2024-05-02 01:47:48,118 DEBUG   [main] Add executor fetch
2024-05-02 01:47:48,119 INFO    [rdr] Running addons/rook-operator/fetch
2024-05-02 01:47:48,127 INFO    [rdr] Running addons/rook-toolbox/fetch
2024-05-02 01:47:48,127 INFO    [rdr] Running addons/rook-cluster/fetch
2024-05-02 01:47:48,137 INFO    [rdr] Running addons/ocm-controller/fetch
2024-05-02 01:47:48,217 DEBUG   [rdr] Fetching /home/abshakya/.cache/drenv/addons/rook-operator.yaml
2024-05-02 01:47:48,237 DEBUG   [rdr] Fetching /home/abshakya/.cache/drenv/addons/rook-cluster.yaml
2024-05-02 01:47:48,240 DEBUG   [rdr] Fetching /home/abshakya/.cache/drenv/addons/ocm-controller.yaml
2024-05-02 01:47:48,243 DEBUG   [rdr] Fetching /home/abshakya/.cache/drenv/addons/rook-toolbox.yaml
2024-05-02 01:47:48,401 INFO    [rdr] addons/rook-toolbox/fetch completed in 0.27 seconds
2024-05-02 01:47:48,417 INFO    [rdr] addons/rook-cluster/fetch completed in 0.29 seconds
2024-05-02 01:47:49,738 INFO    [rdr] addons/rook-operator/fetch completed in 1.62 seconds
2024-05-02 01:48:20,869 INFO    [rdr] addons/ocm-controller/fetch completed in 32.73 seconds
2024-05-02 01:48:20,870 INFO    [rdr] Fetching finishied in 32.75 seconds
2024-05-02 01:48:20,871 DEBUG   [main] Starting shutdown
2024-05-02 01:48:20,871 DEBUG   [main] Shutting down executor fetch
2024-05-02 01:48:20,871 DEBUG   [main] Terminating process group 126967

Fetching resources may take different amount of time for different runs. And once fetched, addons can directly use these resources to get started, saving time and escaping the flaky network situation.

@abhijeet219 abhijeet219 marked this pull request as ready for review April 22, 2024 14:20
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

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

Looks good, but we need to be consistent with cache names.

from drenv import cache

os.chdir(os.path.dirname(__file__))
path = cache.path("addons/cluster-test.yaml")
Copy link
Member

Choose a reason for hiding this comment

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

This is the cache of the rook-cluster addon, so we should name it "addons/rook-cluster.yaml"

test/addons/rook-cluster/start Outdated Show resolved Hide resolved
test/addons/rook-toolbox/fetch Outdated Show resolved Hide resolved
test/addons/rook-toolbox/start Outdated Show resolved Hide resolved
@abhijeet219 abhijeet219 changed the title Caching Rook addons resources on top of PR #1346 Caching Rook addons resources May 1, 2024
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

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

Code looks good, commit message needs re-wrapping.

test/addons/rook-cluster/fetch Show resolved Hide resolved
@nirs
Copy link
Member

nirs commented May 1, 2024

@abhijeet219 Also update the pr message:

  • Reference to Configure rbd-mirror debug logs dynamically #1346 is not relevant now
  • When you show running the adding with and without caching, it is not clear if the addon was run on a started environment, and most of the time is spent on fetching the resources. Please make it more clear what is tested.
  • Showing only drenv fetch ... run will make this much more clear, since we measure only the time to get fetch the resources, and it is also logged in detail if using -v.

@nirs
Copy link
Member

nirs commented May 1, 2024

Tested using #1359

@abhijeet219
Copy link
Member Author

abhijeet219 commented May 2, 2024

@abhijeet219 Also update the pr message:

  • Reference to Configure rbd-mirror debug logs dynamically #1346 is not relevant now
  • When you show running the adding with and without caching, it is not clear if the addon was run on a started environment, and most of the time is spent on fetching the resources. Please make it more clear what is tested.
  • Showing only drenv fetch ... run will make this much more clear, since we measure only the time to get fetch the resources, and it is also logged in detail if using -v.

Added and explained drenv fetch ... in the PR message
Should I remove the starting an addon part from the PR message, it is also making the PR message look very long.

@nirs
Copy link
Member

nirs commented May 2, 2024

Added and explained drenv fetch ... in the PR message Should I remove the starting an addon part from the PR message, it is also making the PR message look very long.

@abhijeet219 yes, the current example are long and not clear and better remove. Describing how fetch works is not needed in this PR - it was needed only in the PR introducing the cache.

Currently the addons fetch kustomization resources whenever they
are started.
This change will start using cache for those resources, so starting
an addon can directly use the cached resources.

Changes:
- drenv fetch can be used to fetch resources anytime.
- Starting an addon will first try to fetch resources, then apply
  the fetched resources. If there is no change, fetch won't do
  anything, so takes very less time.

Signed-off-by: Abhijeet Shakya <[email protected]>
@nirs nirs merged commit bd64a63 into RamenDR:main May 2, 2024
16 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