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

Invalidate registration cache only for affected sidecars #13514

Merged

Conversation

thll
Copy link
Contributor

@thll thll commented Sep 21, 2022

When a configuration is saved, and its tags have changed, we used to invalidate the registration cache for all sidecars.

We now check which sidecars are potentially affected by the change by looking at their tags, and invalidate the cache only for those.

There is still the case where a change to the tags of a configuration doesn't result in a change of the assignment. E.g. if a sidecar has the tags ["apache-logs", "dns-logs"] and we change the tags of a configuration from ["apache-logs"] to ["dns-logs"]. In that case, we wouldn't need to invalidate the cache because the config will still be assigned after the change. But in favour of keeping things simple, we don't try to prevent this.

@thll thll requested a review from mpfz0r September 21, 2022 13:03
@thll thll marked this pull request as draft September 21, 2022 15:09
@thll
Copy link
Contributor Author

thll commented Sep 21, 2022

Needs adjustment after #13504

The cache key has been changed since this PR has been started.
Also use symmetric difference of tags to further reduce
unnecessary invalidations
@thll thll marked this pull request as ready for review September 22, 2022 07:24
Copy link
Contributor

@mpfz0r mpfz0r left a comment

Choose a reason for hiding this comment

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

we could add this optimization also to createConfiguration()

@mpfz0r
Copy link
Contributor

mpfz0r commented Sep 22, 2022

other than that looks great 👍

@thll thll requested a review from mpfz0r September 22, 2022 09:17
@mpfz0r mpfz0r merged commit ac799b1 into sidecar-multi-config-and-tags Sep 22, 2022
@mpfz0r mpfz0r deleted the sidecar-improve-cache-invalidation branch September 22, 2022 13:37
gally47 pushed a commit that referenced this pull request Sep 29, 2022
* Invalidate registration cache only for affected sidecars

* Use node-id for cache invalidation

The cache key has been changed since this PR has been started.

* Consider OS for cache invalidation

Also use symmetric difference of tags to further reduce
unnecessary invalidations

* Invalidate less when creating new configs
gally47 pushed a commit that referenced this pull request Sep 29, 2022
* Invalidate registration cache only for affected sidecars

* Use node-id for cache invalidation

The cache key has been changed since this PR has been started.

* Consider OS for cache invalidation

Also use symmetric difference of tags to further reduce
unnecessary invalidations

* Invalidate less when creating new configs
mpfz0r added a commit that referenced this pull request Oct 19, 2022
…13433)

* feature branch for multiple configs per collector and tags

* Add "tags" field to DTOs (#13367)

* Add spooldir variable to sidecars and their default collector configs (#13349)

* Add support for multiple configurations per collector

This will need a new sidcar release.
Old sidecars are still supported with just a single configuration.

* Use Java 9 Lists

* Add support for ${sidecar.spoolDir} variable

* Fix tests

* Revert "Add support for multiple configurations per collector"

This reverts commit 0a17743.

* Use ${sidecar.spoolDir} for fresh collector templates

And add explanation to the UI

* make eslint happy

* convert tags to Set and fix Autovalue

* Support configuration_id in CollectorStatus

Co-authored-by: Othello Maurer <[email protected]>

* Sidecar tagged assignments (#13409)

* add assignedFromTags property to assignment

* Don't fail for unknown properties on NodeDetails

This makes it easier to extend the sidecar
in the future

* Create sidecar config assignments based on tags

This is a first naive approach.
Every registration request will rebuild the assignments
and this is probably too expensive.

* fix test

* Fix more Tests

* Fine grained EtagService

Use three different caches for configs, collectors and assignments.

* Add Etag caching for UpdateRegistration results

The response to the registration PUT request contains
configuration assignments, which can be cached as well.

This requires us to invalidate the sidecar etag cache in more
situations:
 - Configration assignments
 - User triggered collector actions (stop, start, restart)

* Cleanup old assignment caching commit

* optimize

* fix after merge

* replace deprecated notempty annotation

* cache assignments per sidecar id

using just the md5 of an assignment result might
leed to sidecars missing out on updates.
An assignment is always meant for a single sidecar, caching the entire
assignment over all sidecars is wrong.

A per sidecar cache entry allows more fine grained cache invalidations.

Bump the cacheMaxSize to 5000 entries, which is a more meaningful
number, considered that this might have to hold all sidecars.

* Refactor sidecar register() call.

Only update the tagged assignments when we miss the cache.

* change cache naming

* more refactoring

* improve cache invalidation

* use fancy java

Co-authored-by: Othello Maurer <[email protected]>

* Add more cases for tag invalidation and fix tests

* add tags to Config summary

* Generate EntityTags with a better hash algorithm

As @thll noticed, Object.hashCode() is prone to having collisions

* Handle manual assignments via API

* rename

* Make tags available as config variables.

This allows configs to be written with conditionals.
E.g.:

```
<#if sidecar.tags.apache??>
 - /var/log/apache/*.log
</#if>
```

* dont lose tags on UI configuration updates

Co-authored-by: Othello Maurer <[email protected]>

* Sidecar tags tests (#13502)

* Only assign tagged configs that match the OS

* rename test

* add tests for SidecarService.updateTaggedConfigurationAssignments()

* add tests for SidecarService.applyManualAssignments()

* Fix cache invalidation for collector actions (#13504)

* Fix cache invalidation for collector actions

The registration cache was using the sidecar "_id",
while the ActionService was invalidating the sidecar "node_id".

Change the entire cache to using the "node_id"

* Rename parameter from sidecarId to sidecarNodeId

Co-authored-by: Othello Maurer <[email protected]>

* Invalidate registration cache only for affected sidecars (#13514)

* Invalidate registration cache only for affected sidecars

* Use node-id for cache invalidation

The cache key has been changed since this PR has been started.

* Consider OS for cache invalidation

Also use symmetric difference of tags to further reduce
unnecessary invalidations

* Invalidate less when creating new configs

* Add description for tag variables

* Add "tags" field to DTOs (#13367)

* show CollectorProcessControl as an action button

* added CollectorConfigurationModal

* made search button in SearchForm optional

* Add spooldir variable to sidecars and their default collector configs (#13349)

* Add support for multiple configurations per collector

This will need a new sidcar release.
Old sidecars are still supported with just a single configuration.

* Use Java 9 Lists

* Add support for ${sidecar.spoolDir} variable

* Fix tests

* Revert "Add support for multiple configurations per collector"

This reverts commit 0a17743.

* Use ${sidecar.spoolDir} for fresh collector templates

And add explanation to the UI

* make eslint happy

* convert tags to Set and fix Autovalue

* Support configuration_id in CollectorStatus

Co-authored-by: Othello Maurer <[email protected]>

* handle edge case very long config names

* handle edge cases when its not possible to configure

* change config list styling

* make CollectorConfigurationModal a reusable component

* added CollectorConfigurationModal in CollectorsAdministrationActions

* icon btn as edit btn progress

* Sidecar tagged assignments (#13409)

* add assignedFromTags property to assignment

* Don't fail for unknown properties on NodeDetails

This makes it easier to extend the sidecar
in the future

* Create sidecar config assignments based on tags

This is a first naive approach.
Every registration request will rebuild the assignments
and this is probably too expensive.

* fix test

* Fix more Tests

* Fine grained EtagService

Use three different caches for configs, collectors and assignments.

* Add Etag caching for UpdateRegistration results

The response to the registration PUT request contains
configuration assignments, which can be cached as well.

This requires us to invalidate the sidecar etag cache in more
situations:
 - Configration assignments
 - User triggered collector actions (stop, start, restart)

* Cleanup old assignment caching commit

* optimize

* fix after merge

* replace deprecated notempty annotation

* cache assignments per sidecar id

using just the md5 of an assignment result might
leed to sidecars missing out on updates.
An assignment is always meant for a single sidecar, caching the entire
assignment over all sidecars is wrong.

A per sidecar cache entry allows more fine grained cache invalidations.

Bump the cacheMaxSize to 5000 entries, which is a more meaningful
number, considered that this might have to hold all sidecars.

* Refactor sidecar register() call.

Only update the tagged assignments when we miss the cache.

* change cache naming

* more refactoring

* improve cache invalidation

* use fancy java

Co-authored-by: Othello Maurer <[email protected]>

* Add more cases for tag invalidation and fix tests

* add tags to Config summary

* Generate EntityTags with a better hash algorithm

As @thll noticed, Object.hashCode() is prone to having collisions

* Handle manual assignments via API

* rename

* Make tags available as config variables.

This allows configs to be written with conditionals.
E.g.:

```
<#if sidecar.tags.apache??>
 - /var/log/apache/*.log
</#if>
```

* dont lose tags on UI configuration updates

Co-authored-by: Othello Maurer <[email protected]>

* renamed btn Assign Configurations

* fix wrong postion of ColorLabel

* disable eslint errors

* added ModalSubTitle styling

* Sidecar tags tests (#13502)

* Only assign tagged configs that match the OS

* rename test

* add tests for SidecarService.updateTaggedConfigurationAssignments()

* add tests for SidecarService.applyManualAssignments()

* Fix cache invalidation for collector actions (#13504)

* Fix cache invalidation for collector actions

The registration cache was using the sidecar "_id",
while the ActionService was invalidating the sidecar "node_id".

Change the entire cache to using the "node_id"

* Rename parameter from sidecarId to sidecarNodeId

Co-authored-by: Othello Maurer <[email protected]>

* Invalidate registration cache only for affected sidecars (#13514)

* Invalidate registration cache only for affected sidecars

* Use node-id for cache invalidation

The cache key has been changed since this PR has been started.

* Consider OS for cache invalidation

Also use symmetric difference of tags to further reduce
unnecessary invalidations

* Invalidate less when creating new configs

* added logic to CollectorConfigurationModal to support multiple sidecars

* onReset resets the search form too

* updated confirmConfigurationChange fnct to support multiple sidecars

* simplify Configuration summary when multiple sidecars are selected

* finalized confirmation summary

* fixed exit dialog by ESC key bug

* Add description for tag variables

* added Configuration Tags field in Collector Configuration page

* prototype: implement auto assigned tags in config selection modal

* finalized tags UI implementation

* fix linter issues

* fix sidecars.node_name type error

* handled case assigned_from_tags with multiple sidecarse

* hide close icon btn when isAssignedFromTags

* fixed TemplatesHelper linter issues

* cleanup

* split CollectorConfigurationModal into 2 components to reduce complexity

* small css changes

* added tests for CollectorConfigurationModal

* fixed too long logic lines

* avoid single charachter variables

* fixed Missing semicolons

* removed React. in hooks

* RegExp instance set into a variable

* destruct props

* used styled components

* css styled component changes

* destruct CollectorsAdministrationActions props

* convert CollectorProcessControl to typescript and using React functional component

* removed React. in hooks

* convert CollectorsAdministration to typescript and using React functional component

* reset file

* convert CollectorsAdministrationActions to typescript and using React functional component

* added types

* fix eslint issues

* typed CollectorProcessControl props

* typed CollectorsAdministration props

* typed CollectorsAdministrationActions props

* typed function args and states

* converted ConfigurationTagsSelect to typescript and  functional component

* converted TemplatesHelper to typescript and functional component

* converted ConfigurationForm to typescript and functional component

* removed unused function

* fixed config creation form

* removed read only from types

* used Proptypes.shape for configuration

* fixed console error in SidecarNewConfigurationPage

* added type SidecarCollectorPairType

* make sure the filterQuery handles case error

* made tags field required

* ColorLabel using className instead of inline style

* converted inline-styling to styled-component in SearchForm

* converted CollectorsAdministration.css into styled-components

* addedstyled component for ColorLabel

* added SidecarCollectorConfigurationFacade.java changes

Co-authored-by: Marco Pfatschbacher <[email protected]>
Co-authored-by: Othello Maurer <[email protected]>
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