[ 
https://issues.apache.org/jira/browse/PHOENIX-2535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15217347#comment-15217347
 ] 

Enis Soztutar commented on PHOENIX-2535:
----------------------------------------

Thanks [~sergey.soldatov] for pushing this. 

Some comments: 
 - I think you did reformat the whole pom file. It makes it very hard to review 
the difference that is added in the patch. Can you please undo that. Looking at 
some pom.xml files, unlike the java code, 2 space indentation seems to be used. 
 - Instead of 
{code}
+                                    <pattern>com</pattern>
+                                    
<shadedPattern>shaded.phoenix.com</shadedPattern>
{code}
I think we should use {{org.apache.phoenix.shaded.com}}. Similar for others. 
-   Did a dir listing for the phoenix shaded client jar: 
  -- {{javax/}}, {{com.sum}} contents seems to be fine. 
  -- {{org/apache/thrift}},  {{org/objectweb/}}, {{org/apache/directory}}, 
{{org/eclipse/}} we are not shading these? 
  -- org/mortbay  hbase shades this as well. Not log4j though. Maybe needed for 
slf4j? 
  -- {{sqlline}} seems to have classes around. Needed for it to work? 
  -- We have some of these lying around (from some dependencies). Not sure 
whether they are needed or not:
{code}
META-INF/maven/org.apache.twill/
META-INF/maven/org.apache.twill/twill-common/
META-INF/maven/org.apache.twill/twill-common/pom.xml
META-INF/maven/org.apache.twill/twill-common/pom.properties
{code}
 - HBase does not shade the htrace dependency. I think it maybe relevant 
(phoenix trace -> hbase trace -> hdfs trace in the same context). 
 - Did you test it with running the server jars with hbase? I fear if HBase has 
an internal API that exposes something like guava, and then we are using it in 
Phoenix, it will be a runtime exception I think. One safe option is to 
not-shade server jars, but shade the client jars if it does not work. 
 - {{apache/commons/csv/}} is this part of the API (sorry did not check). If we 
allow extending CSVBulkLoad MR job for example. 
 - flume probably should not be shaded. Otherwise the phoenix-flume will not 
work (phoenix implements flume APIs). 
 - same thing for pig (although I see both shaded and un-shaded pig classes) 


> Create shaded clients (thin + thick) 
> -------------------------------------
>
>                 Key: PHOENIX-2535
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-2535
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: Enis Soztutar
>            Assignee: Sergey Soldatov
>             Fix For: 4.8.0
>
>         Attachments: PHOENIX-2535-1.patch, PHOENIX-2535-2.patch, 
> PHOENIX-2535-3.patch
>
>
> Having shaded client artifacts helps greatly in minimizing the dependency 
> conflicts at the run time. We are seeing more of Phoenix JDBC client being 
> used in Storm topologies and other settings where guava versions become a 
> problem. 
> I think we can do a parallel artifact for the thick client with shaded 
> dependencies and also using shaded hbase. For thin client, maybe shading 
> should be the default since it is new? 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to