Bug 819897 - platform recursive groups include non-committed resources as implicit members
Summary: platform recursive groups include non-committed resources as implicit members
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RHQ Project
Classification: Other
Component: Monitoring
Version: 4.4
Hardware: Unspecified
OS: Unspecified
urgent
unspecified
Target Milestone: ---
: RHQ 4.5.0
Assignee: John Mazzitelli
QA Contact: Mike Foley
URL:
Whiteboard:
Depends On: 820981 841026
Blocks: jon310-sprint11, rhq44-sprint11 820432
TreeView+ depends on / blocked
 
Reported: 2012-05-08 14:34 UTC by Heiko W. Rupp
Modified: 2013-09-01 10:12 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 820432 (view as bug list)
Environment:
Last Closed: 2013-09-01 10:12:09 UTC
Embargoed:


Attachments (Terms of Use)
Screenshot (15.95 KB, image/png)
2012-05-08 14:34 UTC, Heiko W. Rupp
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 815341 0 urgent CLOSED groups display availability as green when member resources are in unknown state 2021-02-22 00:41:40 UTC
Red Hat Bugzilla 820981 0 unspecified CLOSED fix group queries to not return non-committed resources 2021-02-22 00:41:40 UTC

Internal Links: 815341 820981

Description Heiko W. Rupp 2012-05-08 14:34:23 UTC
Created attachment 583011 [details]
Screenshot

[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

Comment 1 Heiko W. Rupp 2012-05-08 15:19:46 UTC
Those 6 unknown resources are sitting in the discovery portlet as ignored

Comment 3 John Mazzitelli 2012-05-08 15:37:19 UTC
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.

Comment 4 John Mazzitelli 2012-05-08 16:05:30 UTC
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.

Comment 5 John Mazzitelli 2012-05-08 16:10:11 UTC
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.

Comment 6 John Mazzitelli 2012-05-08 19:53:59 UTC
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.

Comment 7 John Mazzitelli 2012-05-09 14:44:20 UTC
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).

Comment 8 John Mazzitelli 2012-05-09 14:48:34 UTC
(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.

Comment 9 Heiko W. Rupp 2012-05-09 14:52:59 UTC
If we just close, we at least need to document this

Comment 10 Charles Crouch 2012-05-09 16:48:53 UTC
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.

Comment 11 John Mazzitelli 2012-05-09 19:43:11 UTC
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.

Comment 12 John Mazzitelli 2012-05-09 19:54:52 UTC
(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 :)

Comment 13 John Mazzitelli 2012-05-09 20:28:26 UTC
git commit to master: 2e47850

this fixes the left-hand tree in the group view - it doesn't show non-committed resources anymore

Comment 15 John Mazzitelli 2012-05-11 13:29:04 UTC
(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.

Comment 16 John Mazzitelli 2012-05-11 19:54:08 UTC
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.

Comment 17 John Mazzitelli 2012-05-11 20:10:33 UTC
(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

Comment 18 John Mazzitelli 2012-05-11 20:15:38 UTC
opps.. commit 3f5f02c489179e81d783b49d8270ae869b74d404 fixes a test method name that I forgot to change

Comment 19 John Mazzitelli 2012-05-14 15:15:39 UTC
git commit master: 5c02dfb fixes old unit tests that now have to make sure new resources created are in committed inventory state

Comment 20 Heiko W. Rupp 2013-09-01 10:12:09 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.