Created attachment 583011 [details]
[16:30:06] <@pilhuhn> I think I found a bug with availability counting . I have created a recursive comp group with my platform as root
[16:30:51] <jshaughn> mazz ^ take note
[16:31:27] <@pilhuhn> all resources reports 439 resources (as does the inv overview portlet). Going to the comp. group tab, it shows 437 as up, 1 as down, 1 as disabled and 6 as unknown
[16:31:35] <@pilhuhn> the 437/1/1 are correct
[16:32:23] <@pilhuhn> but the 6 unknown are not
Those 6 unknown resources are sitting in the discovery portlet as ignored
this is probably due to the resources are not yet committed. though the question is - how can non-committed resources get into the group in the first place.
here's the issue after thinking about it and reading the description of the problem.
When we create a platform recursive group (and specifically I am talking about a PLATFORM recursive group, as the description of this BZ says) we are most likely adding in all children even those we have not yet committed (i.e. the top-level servers will still be in the discovery queue).
I am pretty sure those group queries do not filter out non-committed resources.
If you try this with a server, this won't happen because you can't have uncommitted resources under a committed server. But you CAN have uncommitted child servers under a committed platform.
Therefore, this issue is limited in scope to only platform groups that are ALSO recursive groups. Anything other than a platform-recursive group won't exhibit this issue.
Now, the question then becomes - what do we do in this case?
It could be argued we are showing the correct results - because you have not officially imported those top-level child servers in inventory, we do NOT know what the availability state of those non-committed servers are - hence, showing UNKNOWN could be a valid thing to show.
It could equally be argued we should not show anything - not even UNKNOWN - because those uncommitted resources shouldn't even be shown in the UI.
Its possible the simplest thing to do would be to filter out non-committed resources in the group queries. I ran a whole series of performance tests on these queries - before we add in changes to the queries, we need to again perform performance tests to make sure an additional WHERE clause doesn't make the queries worse.
see bug 815341 - this is the issue where we added the explicit showing of the UNKNOWN resources and changed the group queries to be able to do so. Its those queries we changed that could use the new WHERE clause to filter out the uncommitted resources.
1. Start with fresh db, start server and agent
2. Import only the platform but not ALL of the top level servers
3. Create a new compatible recursive group - add only the platform resource
4. Navigate the group's view
Notice the group's availability icon is the yellow triangle. Notice the left
hand tree shows all the platform's children - even the top-level servers you
have not yet committed. And, as this BZ originally indicated, there will be a
non-zero number of UNKNOWN resources in the group list view.
So, the issue is more general than just the avail count. The issue is - the
recursive group's implicit resources include those resources not yet committed
(i.e. have the NEW inventory status).
The question we have to ask is - what do we do with uncommitted resources?
Today, we include them in the implicit members list. If we do NOT include
non-committed resources when we initially create the group - what do we do when
we later commit some of those resources? We would need to add them to the
implicit members list for all recursive groups that its parent belongs to.
I just tested on JON 3.0.1 (based on an older RHQ version) and confirmed this behavior is nothing new. Even back then, anytime we created a platform recursive group, we also added non-committed top-level child resources as implicit members to the group.
So this is not a regression. But as part of this BZ, we should consider if that is the behavior we still want.
We could leave it as-is and close this as NOTABUG.
We could NOT add non-committed resources to the group (but this then adds the problem of what happens when we later DO commit those resources - we have to remember to add them as implicit members to all recursive groups where its parent platform is a member).
We could change the queries to filter out non-committed resources (e.g. in the query that produces the left-hand tree of the group view).
(In reply to comment #7)
> So this is not a regression.
I want to backtrack on that claim a little bit. The group list view (e.g. the snapshot that heiko attached) DOES in fact (in the older versions) show the resources as green/UP even though they are uncommitted. So, today the behavior is different in that you will now see these uncommitted resources as UNKNOWN as opposed to GREEN.
That said, it could be argued that the way we do it now (showing it UNKNOWN) is more appropriate than how it was before (showing GREEN) since showing GREEN implies everythign is OK and normal (when in fact, how can a resource be normal when it isn't even imported into inventory?)
In any event, I still think we need a discussion to determine what we want the behavior to be in the case when we have a platform recursive group and uncommitted resources under that platform.
If we just close, we at least need to document this
I don't see there is much need for discussion, we shouldn't *ever* be showing
uncommitted resources in the regular inventory. The only place uncommitted
resources should appear is in the Import related resource lists.
git commit to master: 4e1a360
this commit fixes the issue this BZ reported - the count in the group list no longer shows counts for committed resources. the query filters out non-committed resources.
still need to hide the resources from the left-hand tree.
(In reply to comment #11)
> git commit to master: 4e1a360
> this commit fixes the issue this BZ reported - the count in the group list no
> longer shows counts for committed resources. the query filters out
> non-committed resources.
> still need to hide the resources from the left-hand tree.
obviously, I meant to say "no longer shows counts for NON-committed resources" - but just to be clear, I'll point that typo out :)
git commit to master: 2e47850
this fixes the left-hand tree in the group view - it doesn't show non-committed resources anymore
(In reply to comment #10)
> I don't see there is much need for discussion, we shouldn't *ever* be showing
> uncommitted resources in the regular inventory. The only place uncommitted
> resources should appear is in the Import related resource lists.
There are many places where our group queries aren't filtering out non-committed resources. I created bug 820981 to address this.
git commit 2e47850 has a problem in it (eres.inventoryStatus should be eres.inventory_status). I will be fixing this as part of bug 820981. That bug will block this one.
(In reply to comment #16)
> git commit 2e47850 has a problem in it (eres.inventoryStatus should be
> eres.inventory_status). I will be fixing this as part of bug 820981. That bug
> will block this one.
git commit to master 3d1a2e5 fixes this issue and adds some unit tests
opps.. commit 3f5f02c489179e81d783b49d8270ae869b74d404 fixes a test method name that I forgot to change
git commit master: 5c02dfb fixes old unit tests that now have to make sure new resources created are in committed inventory state
Bulk closing of items that are on_qa and in old RHQ releases, which are out for a long time and where the issue has not been re-opened since.