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

Enable SPIRE Agent LRU cache by default #4773

Merged
merged 5 commits into from
Jan 20, 2024

Conversation

prasadborole1
Copy link
Contributor

Addresses #4224

Signed-off-by: Prasad Borole <prasadb@uber.com>
rturner3
rturner3 previously approved these changes Jan 3, 2024
@@ -468,6 +469,11 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
ac.UseSyncAuthorizedEntries = c.Agent.Experimental.UseSyncAuthorizedEntries
ac.X509SVIDCacheMaxSize = c.Agent.Experimental.X509SVIDCacheMaxSize

if c.Agent.Experimental.DisableLRUCache && ac.X509SVIDCacheMaxSize != 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it really experimental disablig LRU cache?

Copy link
Collaborator

Choose a reason for hiding this comment

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

may we add documentation about this configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the README.

Signed-off-by: Prasad Borole <prasadb@uber.com>
| `named_pipe_name` | Pipe name to bind the SPIRE Agent API named pipe (Windows only) | \spire-agent\public\api |
| `sync_interval` | Sync interval with SPIRE server with exponential backoff | 5 sec |
| `x509_svid_cache_max_size` | Soft limit of max number of SVIDs that would be stored in LRU cache | 1000 |
| `disable_lru_cache` | Disables the SPIRE Agent LRU cache used for storing SVIDs | false |
Copy link
Collaborator

Choose a reason for hiding this comment

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

doc looks good, but I'm worried if people will understand what is the difference disabling it, can you explan a little more about how it will affect? maybe telling that it go back to old approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the description to convey that it will use non lru cache. cc @azdagron

@azdagron
Copy link
Member

@prasadborole1 any chance you can fix up the documentation as suggested by Marcos? Let us know if this is something you don't have time for and we'll get someone else to do it. Thanks!

Signed-off-by: Prasad Borole <prasadb@uber.com>
@MarcosDY MarcosDY added this to the 1.9.0 milestone Jan 20, 2024
@MarcosDY MarcosDY merged commit 01e10be into spiffe:main Jan 20, 2024
32 checks passed
faisal-memon pushed a commit to faisal-memon/spire that referenced this pull request Feb 7, 2024
* Enable SPIRE Agent LRU cache by default

Signed-off-by: Prasad Borole <prasadb@uber.com>
Signed-off-by: Faisal Memon <fymemon@yahoo.com>
sriyer pushed a commit to spire-vault/spire that referenced this pull request Feb 23, 2024
* Enable SPIRE Agent LRU cache by default

Signed-off-by: Prasad Borole <prasadb@uber.com>
rushi47 pushed a commit to rushi47/spire that referenced this pull request Apr 11, 2024
* Enable SPIRE Agent LRU cache by default

Signed-off-by: Prasad Borole <prasadb@uber.com>
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.

4 participants