Jira (PUP-10713) Fine grained environment timeout issues

36 views
Skip to first unread message

Josh Cooper (Jira)

unread,
Oct 13, 2020, 8:26:03 PM10/13/20
to puppe...@googlegroups.com
Josh Cooper created an issue
 
Puppet / Bug PUP-10713
Fine grained environment timeout issues
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/10/13 5:25 PM
Priority: Normal Normal
Reporter: Josh Cooper

Puppet's environment cache has several methods for evicting an environment, but they do not mutate the cache in a consistent way:

1. Whenever an environment is requested, we evict all environments that are now expired, but the evict_if_expired method doesn't update the @next_expiration instance variable. As a result, the optimization in clear_all_evicted may cause us to call the cache_expiration_service.expired?(name.to_sym twice.

2. The clear_all method does not notify the cache_expiration_service that it evicted all of the environments. This could lead to puppetserver holding onto environment names unnecessarily. The method also does not clear environment related settings (for each environment).

3. The clear method does not remove the environment entry from the @expirations sorted set, it doesn't recalculate the @next_expiration time, or clear the enviroment related settings.

The clear_all and clear methods do not appear to be used in puppet, as it was decided earlier to only support a REST API for cache eviction. So we might want to remove clear_all and clear to avoid confusion.
Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Josh Cooper (Jira)

unread,
Oct 14, 2020, 1:50:03 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Labels: platform_7

Josh Cooper (Jira)

unread,
Oct 14, 2020, 1:51:02 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10713
 
Re: Fine grained environment timeout issues

Added to platform_7 so that we can delete the clear and clear_all methods (so all manual cache eviction is handled through the REST API).

Josh Cooper (Jira)

unread,
Oct 14, 2020, 1:51:03 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Team: Coremunity

Josh Cooper (Jira)

unread,
Oct 16, 2020, 1:31:03 PM10/16/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Epic Link: PUP-10593

Josh Cooper (Jira)

unread,
Oct 22, 2020, 8:52:02 PM10/22/20
to puppe...@googlegroups.com
Josh Cooper assigned an issue to Josh Cooper
Change By: Josh Cooper
Assignee: Josh Cooper

Josh Cooper (Jira)

unread,
Oct 22, 2020, 8:52:03 PM10/22/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Platform Core KANBAN

Josh Cooper (Jira)

unread,
Oct 22, 2020, 8:52:03 PM10/22/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 7.0.0

Josh Cooper (Jira)

unread,
Nov 4, 2020, 7:29:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Puppet's environment cache has several methods for evicting an environment, but they do not mutate the cache in a consistent way:

1. Whenever an environment is requested, we evict all environments that are now expired, but the {{evict_if_expired}} method doesn't update the {{@next_expiration}} instance variable. As a result, the optimization in {{clear_all_evicted}} may cause us to call the {{cache_expiration_service.expired?(name.to_sym}} twice.

2. The {{clear_all}} method does not notify the {{cache_expiration_service}} that it evicted all of the environments. This could lead to puppetserver holding onto environment names unnecessarily. The method also does not clear environment related settings (for each environment).

3. The {{clear}} method does not remove the environment entry from the {{@expirations}} sorted set, it doesn't recalculate the {{@next_expiration}} time, or clear the enviroment related settings.

The - {{clear_all}} and - {{clear}} methods do does not appear to be used in puppet, as it was decided earlier to only support a REST API for cache eviction. - So we might want to remove {{clear_all}} and {{clear}} to avoid confusion. - The {{clear_all}} method is used when Puppet[:rich_data] is set and also in unit tests.

Josh Cooper (Jira)

unread,
Nov 4, 2020, 7:29:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Epic Link: PUP-10593

Josh Cooper (Jira)

unread,
Nov 4, 2020, 7:29:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.20.0

Josh Cooper (Jira)

unread,
Nov 4, 2020, 7:44:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Puppet's environment cache has several three methods for evicting an a single expired environment, all expired environments, or all environments, but they do not don't mutate the cache in a consistent way:

1. Whenever {{Puppet::Environments::Cached#get_conf(name)}} to lookup the {{EnvironmentConf}} for an environment is requested , we evict all environments clear that are environment if it is now expired , but the via {{evict_if_expired (name) }} , but the method doesn't update the {{@next_expiration}} instance variable. As a result, the optimization in {{clear_all_evicted}} may cause us to call the {{cache_expiration_service.expired?(name.to_sym}} twice.

2. The Whenever {{ clear_all Puppet::Environments::Cached#get(name) }} is called, we clear all environments that are now expired via {{clear_all_expired}}. This method does not notify updates the cache consistently.

3. Whenever {{ Puppet::Environments::Cached#clear_all}} is called, the {{ cache_expiration_service}} that it evicted all is never notified of the environments. This eviction, which could lead to puppetserver holding onto environment names unnecessarily in memory . The method also does not clear environment related settings (for each environment). So anyone calling `Puppet[:rich_data]` for example could "see" the value from the now evicted environment. The {{clear_all}} method is used when Puppet[:rich_data] is set and also in unit tests.

3 4 . The {{ Puppet::Environments::Cached# clear}} method does not remove the environment entry from the {{@expirations}} sorted set , it and doesn't recalculate the {{@next_expiration}} time, or clear the enviroment related settings.

The - {{ clear_all}} and- {{ clear}} methods does not appear to be used in puppet, as it was decided earlier to only support a REST API for cache eviction. -So we might want to remove {{clear_all}} and {{clear}} to avoid confusion.- The {{clear_all}} method is used when Puppet[:rich_data] is set and also in unit tests.

Josh Cooper (Jira)

unread,
Nov 4, 2020, 7:55:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Labels: platform_7

Josh Cooper (Jira)

unread,
Nov 4, 2020, 7:55:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 7.0.0

Josh Cooper (Jira)

unread,
Nov 4, 2020, 8:46:03 PM11/4/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Puppet's environment cache has three methods for evicting a single expired environment, all expired environments, or all environments, but they don't mutate the cache in a consistent way:

1. Whenever {{Puppet::Environments::Cached#get_conf(name)}} is called to lookup the {{EnvironmentConf}} for an environment, we clear that environment if it is now expired via {{evict_if_expired(name)}}, but the method doesn't update the {{@next_expiration}} instance variable. As a result, the optimization in {{clear_all_evicted}} may cause us to call the {{cache_expiration_service.expired?(name.to_sym}} twice.

2. Whenever {{Puppet::Environments::Cached#get(name)}} is called, we clear all environments that are now expired via {{clear_all_expired}}. This method updates the cache consistently.

3. Whenever {{Puppet::Environments::Cached#clear_all}} is called, the {{cache_expiration_service}} is never notified of the eviction, which could lead to puppetserver holding onto environment names in memory. The method also does not clear environment related settings (for each environment). So anyone calling `Puppet[:rich_data]` for example could "see" the value from the now evicted environment. The {{clear_all}} method is used when Puppet[:rich_data] is set and also in unit tests.

4. The {{Puppet::Environments::Cached#clear}} method does not remove the environment entry from the {{@expirations}} sorted set and doesn't recalculate the {{@next_expiration}} time, or clear the enviroment related settings. The {{clear}} methods does not appear to be used in puppet, as it was decided earlier to only support a REST API for cache eviction.

Josh Cooper (Jira)

unread,
Nov 13, 2020, 2:06:05 PM11/13/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10713

Josh Cooper (Jira)

unread,
Nov 13, 2020, 4:03:03 PM11/13/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes: Bug Fix
Release Notes Summary: Previously, if the environment.conf for an environment was updated and the environment was cleared, then puppetserver would still use the old values in memory for per-environment settings such as modulepath, config_version, etc. This was true if the environment timed out or if the environment was explicitly cleared using puppetserver's environment cache REST API.

Now if an environment is cleared, puppet will reload the per-environment settings from the updated environment.conf.

Mihai Buzgau (Jira)

unread,
Dec 9, 2020, 7:06:04 AM12/9/20
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Fix Version/s: PUP 7.1.0

Claire Cadman (Jira)

unread,
Dec 9, 2020, 11:40:04 AM12/9/20
to puppe...@googlegroups.com
Claire Cadman updated an issue
Change By: Claire Cadman
Labels: doc_reviewed
Reply all
Reply to author
Forward
0 new messages