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

[WIP] Clear cache of MiqProductFeature to store MiqUserRole #5101

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Dec 18, 2018

Method rbac_compact_features called here is using cashe of feaures
(MiqProductFeature#feature_cache)

It needs to be cleared out when newly added tenant product features
are added.
even when it is cleared after tenant creation.

This is working locally in development mode and I am not sure why it is happening in product mode.
Anyway this PR is fixing this bug.

cc @gtanzillo @skateman

Reproducer:

2rxub5inkr

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1468795

@miq-bot add_label bug, blocker

Method rbac_compact_features is using cashe of for feaures
(MiqProductFeature#feature_cache)

It needs to be cleared out when newly added tenant product features
are added.
@miq-bot
Copy link
Member

miq-bot commented Dec 18, 2018

Checked commit lpichler@f0a608c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@Fryguy
Copy link
Member

Fryguy commented Dec 18, 2018

This feels weird here...IIRC, the purpose of the cache was because features are static. If they are not (thinking about that new dynamic set of features), then I would expect clearing of the cache to be in the model itself, not in the UI.

@lpichler
Copy link
Contributor Author

@Fryguy then are not static now, and clearing is done on model level but it is not working in production mode. Do you have any idea how it could be possible ?

@gtanzillo
Copy link
Member

Is it possible that the code that clears the cache in the model is not being executed in the same process as the UI? Since the clear is tied to a callback. It may be problematic in general when there's more than 1 UI worker

@skateman
Copy link
Member

@gtanzillo yup, models are isolated from the controllers. I don't think you have access to the session from models without explicitly passing it, but I can dig into this if you want a precise answer.

@lpichler
Copy link
Contributor Author

@skateman I want a precise answer :)

@skateman
Copy link
Member

skateman commented Dec 18, 2018

Okay, I checked the object_ids and they're all good so the issue is not there. I noticed that the tenant creation is done in the API, but I'm using rails s so it uses the same process for handling the requests.

I added some debugger breakpoints in the area and determined, that after cleaning the cache in the API request, it truly becomes empty. However, when the next UI request gets fired, the cache has the items as nothing happened.

I was thinking that it has something to do with the session, but I'm not sure how the session is implemented in the API.

# API
pry(#<OpsController>)> session
=> #<ActionDispatch::Request::Session:0x7f7efb4cf410 not yet loaded>
pry(#<OpsController>)> session.object_id
=> 70091679365640

# UI
pry(#<OpsController>)> session
=> #<ActionDispatch::Request::Session:0x00007f7ef82cd408 ...>
pry(#<OpsController>)> session.object_id
=> 70091653147140

The object_ids should be probably different, but in the API the object is not yet loaded. I don't know what it means, but it might be helpful to someone else. So the thing I determined is that if you clean the cache from the API, it doesn't reflects the changes in the UI...

PS: I almost depleted the pool of available Men in Black character names during the testing.
screenshot from 2018-12-18 19-43-04

@lpichler lpichler changed the title Clear cashe of MiqProductFeature to store MiqUserRole [WIP] Clear cashe of MiqProductFeature to store MiqUserRole Dec 19, 2018
@miq-bot miq-bot added the wip label Dec 19, 2018
@lpichler
Copy link
Contributor Author

thanks! and we found other solution #5108

@lpichler lpichler closed this Dec 20, 2018
@lpichler lpichler deleted the clear_cashe_of_MiqProductFeature_to_store_MiqUserRole branch December 20, 2018 19:38
@lpichler lpichler changed the title [WIP] Clear cashe of MiqProductFeature to store MiqUserRole [WIP] Clear cache of MiqProductFeature to store MiqUserRole Dec 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants