Vrushali C commented on YARN-3904:

A couple of more things that came to mind. We need not change the patch for 
just these, but wanted to say what's on my mind.

- Do we want to provide a dropTable api ? I think we should not. In production 
situation, this can be a costly mistake if someone is testing their code on the 
cluster. A drop table should be a very manual command so that one is aware that 
they are running it. 

- Are the '?' and ',' special characters in this line? Is so, we dont have to 
change this right now, but maybe next time this code is being looked at, could 
we make it into a constant 
String sql = "UPSERT INTO " + info.getTableName()
                + " (" + StringUtils.join(info.getPrimaryKeyList(), ",")
                + ", created_time, modified_time, metric_names) "
                + "VALUES ("
                + StringUtils.repeat("?,", info.getPrimaryKeyList().length)
                + "?, ?, ?)";

The patch looks good overall. thanks [~gtCarrera9] 

> Refactor timelineservice.storage to add support to online and offline 
> aggregation writers
> -----------------------------------------------------------------------------------------
>                 Key: YARN-3904
>                 URL: https://issues.apache.org/jira/browse/YARN-3904
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Li Lu
>            Assignee: Li Lu
>         Attachments: YARN-3904-YARN-2928.001.patch, 
> YARN-3904-YARN-2928.002.patch, YARN-3904-YARN-2928.003.patch, 
> YARN-3904-YARN-2928.004.patch, YARN-3904-YARN-2928.005.patch, 
> YARN-3904-YARN-2928.006.patch, YARN-3904-YARN-2928.007.patch, 
> YARN-3904-YARN-2928.008.patch
> After we finished the design for time-based aggregation, we can adopt our 
> existing Phoenix storage into the storage of the aggregated data. In this 
> JIRA, I'm proposing to refactor writers to add support to aggregation 
> writers. Offline aggregation writers typically has less contextual 
> information. We can distinguish these writers by special naming. We can also 
> use CollectorContexts to model all contextual information and use it in our 
> writer interfaces. 

This message was sent by Atlassian JIRA

Reply via email to