Varun Saxena commented on YARN-3862:

createHBaseColQualPrefixFilter(): this is still trying to compute the column 
prefix by hand. The main point of introducing getColumnPrefixBytes() on 
ColumnPrefix was to avoid doing this for confs and metrics. Can we rework the 
signatures of createHBaseFilterList() so that we can rely on 
ColumnPrefix.getColumnPrefixBytes()? Ideally all computations of qualifier 
bytes should go through ColumnPrefix.getColumnPrefixBytes().
What we are trying to do here is to convert prefixes coming in filters from 
client and try to match(prefix match) them against column qualifier.
We store config and metric names directly as column qualifiers without any 
prefix(Except in flow run prefix table). So there is no fixed column prefix for 
configs and metrics anyways.
Let us say, we have column qualifiers (in config column family) as 
mapreduce.map.java.opts, mapreduce.map.memory.mb, mapreduce.reduce.memory.mb, 
Now user may want to query all the map related configurations and may send 
{{mapreduce.map}} as prefix. But he can send an invalid prefix like 
{{mapreduce_map}} as well. So prefixes in createHBaseColQualPrefixFilter() can 
be anything and cannot be fetched via a call to 

bq. I'm not too sure about the name; for other tests we basically combined the 
reader and writer tests. Thoughts on how to make this best fit into the 
existing tests?
Ok. Maybe can move these tests to TestHBaseTimelineStorage. Let me see.

bq. I keep confusing configFilters and confs. 
Maybe confs and metrics can be renamed as configsToRetrieve and 
metricsToRetrieve respectively. Thoughts ?

bq. On a related note, this is probably outside the scope of this JIRA, but I 
see that the configFilter and metricFilter are applied on the client-side.
Yes. This will be handled in YARN-3863. However, event filters, relatesTo and 
isRelatedTo still need to be matched at client side because of the way these 
values are stored in our tables. We can discuss this though.

bq. l.156: Why do we need to check if configFilters == null? 
This will be removed in YARN-3863. This is done because we need to fetch 
configs if we have to match them on client side(As of now till 3863 goes in). 
However we should probably fetch all configs irrespective of confs field if 
match has to be done on client side. This is missed in this patch. This code 
will have to be removed though in 3863.

bq. Related to one of the points above, at least we should add javadoc that 
clearly explains confs and metrics
Agree. Will add.

bq. l.139: nit: typo: releated -> related

Other comments are due to YARN-4053 going in. Will fix them in next version of 
the patch.

> Decide which contents to retrieve and send back in response in TimelineReader
> -----------------------------------------------------------------------------
>                 Key: YARN-3862
>                 URL: https://issues.apache.org/jira/browse/YARN-3862
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-2928
>            Reporter: Varun Saxena
>            Assignee: Varun Saxena
>              Labels: yarn-2928-1st-milestone
>         Attachments: YARN-3862-YARN-2928.wip.01.patch, 
> YARN-3862-YARN-2928.wip.02.patch, YARN-3862-YARN-2928.wip.03.patch, 
> YARN-3862-feature-YARN-2928.wip.03.patch
> Currently, we will retrieve all the contents of the field if that field is 
> specified in the query API. In case of configs and metrics, this can become a 
> lot of data even though the user doesn't need it. So we need to provide a way 
> to query only a set of configs or metrics.
> As a comma spearated list of configs/metrics to be returned will be quite 
> cumbersome to specify, we have to support either of the following options :
> # Prefix match
> # Regex
> # Group the configs/metrics and query that group.
> We also need a facility to specify a metric time window to return metrics in 
> a that window. This may be useful in plotting graphs 

This message was sent by Atlassian JIRA

Reply via email to