> On June 27, 2014, 2:36 p.m., Tom Beerbower wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java, > > lines 326-331 > > <https://reviews.apache.org/r/23127/diff/1/?file=619388#file619388line326> > > > > Just so I understand... you are checking if the predicate contains any > > properties that haven't been set yet by the resource provider, right? It's > > easier than checking all of the property providers. Good idea. > > > > I don't think that subResourcePropertiesInpredicate is a good name. > > The set will contain any properties of the top level resource that aren't > > supported by the resource provider. It is really a set of unsupported > > property ids. > > Dmytro Sen wrote: > Can propose more correct name ?
Since they are unsupported property ids, how about unsupportedPropertyIds. Maybe add a comment to help make it clear ... // if the predicate contains property ids that aren't supported by the resource provider then we need to populate the resource further before the resource can be evaluated against the predicate. Also, I would change the name to populateResourceRequired since it doesn't just apply to sub resource property ids... or am I misunderstanding this? Aren't we checking to see it the resources that we have need to be further populated before we page them? > On June 27, 2014, 2:36 p.m., Tom Beerbower wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java, > > lines 321-322 > > <https://reviews.apache.org/r/23127/diff/1/?file=619389#file619389line321> > > > > This was just to help organize the class methods. I think it helps > > visualize what a class is used for when you can see all of its public > > methods together. I use the headings to try to keep things organized. I'm > > not sure why you want to remove it but whatever... > > Dmytro Sen wrote: > Because now it's overrided from ClusterController interface. > > Dmytro Sen wrote: > All public ClusterControllerImpl now overrided from ClusterController > interface Ok, sorry ... I understand now. Could you then add the @Override annotation and get rid of the doc in the class for ensureResourceProvider? - Tom ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23127/#review46858 ----------------------------------------------------------- On June 27, 2014, 1:40 p.m., Dmytro Sen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23127/ > ----------------------------------------------------------- > > (Updated June 27, 2014, 1:40 p.m.) > > > Review request for Ambari, Sid Wagle and Tom Beerbower. > > > Bugs: AMBARI-6306 > https://issues.apache.org/jira/browse/AMBARI-6306 > > > Repository: ambari > > > Description > ------- > > Slowness of filtering operations on hosts, opening a JIRA to track the > performance aspect of it. > In general, filtering operations available on the Hosts page (such as > filtering by host_status, hostname, ip, etc., are slow and takes more than 40 > seconds on a 2k-node cluster). > > After the patch response time is 1-3 seconds, if there is no subresources in > predicate > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/query/QueryImpl.java > ec8a3d4 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java > 3f8f317 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackDefinedPropertyProvider.java > 51c7565 > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/ClusterController.java > d7d916e > > Diff: https://reviews.apache.org/r/23127/diff/ > > > Testing > ------- > > Fixing > > > Thanks, > > Dmytro Sen > >
