You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/30 05:35:09 UTC

[GitHub] [druid] dongjoon-hyun opened a new pull request #11518: Upgrade ORC to 1.6.9

dongjoon-hyun opened a new pull request #11518:
URL: https://github.com/apache/druid/pull/11518


   ### Description
   
   This PR aims to upgrade Apache ORC from 1.5.10 to 1.6.9 and `licenses.yaml` correspondingly to bring the latest bug fixes and updates.
   
   - https://orc.apache.org/docs/releases.html#current-release---169
   
   Apache ORC 1.5.10 is too old and the following versions are available. Apache ORC community highly recommends to use 1.6.9.
   - 1.6.9: https://orc.apache.org/news/2021/07/02/ORC-1.6.9/
   - 1.5.12: https://orc.apache.org/news/2020/09/30/ORC-1.5.12/
   
   <hr>
   
   This PR has:
   - [v] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [v] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] rohangarg commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
rohangarg commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890130354


   @dongjoon-hyun : Yes, you are right. Apologies, I missed that dependency analysis in druid fails compilation if we're not declaring any directly used dependency (the default maven behavior gives a warning). Maybe that was the original reason to keep it explicit :) I had missed the `-DfailOnWarning=true` flag during local check so the compile had passed for me. 
   Also, I did a quick lookup and the observation you had wrt to the transitive dependencies is correct - the maven page states that `Exclusions work on the entire dependency graph below the point where they are declared.` (https://maven.apache.org/guides/introduction/introduction-to-optional-and-excludes-dependencies.html)
   
   You updated change LGTM - thanks for the tryout and changes! 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890223066


   @dongjoon-hyun sure, I will have a look soon.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890128999


   @rohangarg . Are you sure about your comment? Druid analyze check seems to enforce the declaration.
   > I saw that orc-mapreduce has marked orc-core as its dependency so maybe we could just use that. 
   - https://app.travis-ci.com/github/apache/druid/jobs/528073697
   ```
   [WARNING] Used undeclared dependencies found:
   [WARNING]    org.apache.orc:orc-core:jar:1.6.9:compile
   [INFO] Add the following to your pom to correct the missing dependencies: 
   [INFO] 
   <dependency>
     <groupId>org.apache.orc</groupId>
     <artifactId>orc-core</artifactId>
     <version>1.6.9</version>
   </dependency>
   ```
   
   I'll revert the last commit because this PR already passed before the last commit.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun edited a comment on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890042258


   Oh, I double-checked it. It seems that the exclusion rules of `orc-mapreduce` includes all exclusion rules of `orc-core`. I'll revise this PR according to your advice, @rohangarg . Thank you again. Let's see what CIs say.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson merged pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
jihoonson merged pull request #11518:
URL: https://github.com/apache/druid/pull/11518


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890470281


   Thank you so much, @jihoonson !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890031216


   Hi, @rohangarg . Thank you for review. 
   
   Yes. AFAIK, the `orc-core` dependency is there because we want to cut the transitive dependencies of it. If we use only `orc-mapreduce` dependency and rely on the Maven's transitive dependency resolutions, the global dependency tree might be affected.
   > do we have to include orc-core are a dependency separately (it is so in the current code)? 
   
   ```
           <dependency>
               <groupId>org.apache.orc</groupId>
               <artifactId>orc-core</artifactId>
               <version>${orc.version}</version>
               <exclusions>
                   <exclusion>
                       <groupId>com.google.protobuf</groupId>
                       <artifactId>protobuf-java</artifactId>
                   </exclusion>
                   <exclusion>
                       <groupId>commons-lang</groupId>
                       <artifactId>commons-lang</artifactId>
                   </exclusion>
                   <exclusion>
                       <groupId>javax.xml.bind</groupId>
                       <artifactId>jaxb-api</artifactId>
                   </exclusion>
                   <exclusion>
                       <groupId>org.apache.hadoop</groupId>
                       <artifactId>hadoop-hdfs</artifactId>
                   </exclusion>
                   <exclusion>
                       <groupId>org.slf4j</groupId>
                       <artifactId>slf4j-api</artifactId>
                   </exclusion>
               </exclusions>
           </dependency>
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890042258


   Oh, I double-checked it. It seems that the exclusion rules of `orc-mapreduce` includes all exclusion rules of `orc-core`. I'll revise this PR according to your advice, @rohangarg . Let's see what CIs say.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890131529


   Could you review this PR when you have a time, @jihoonson ?
   This PR already passed at 9698b7d19227d2fe6388d9749a9a576cdeaa5b91 and now I reverted this to that commit.
   Among, the latest two failures,
   1. The dependency change is reverted
   2. The IOException issue will be fixed https://github.com/apache/druid/pull/11522 .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] rohangarg commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
rohangarg commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-889859316


   @dongjoon-hyun - Thanks for the change - I wanted to ask that in the `orc-extensions` pom, do we have to include `orc-core` are a dependency separately (it is so in the current code)? I saw that `orc-mapreduce` has marked `orc-core` as its dependency so maybe we could just use that. I also checked and `orc-core` is only used in orc extension. wdyt? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890131962


   :) I also learned it at this time. Thank you for you reviews and helps, @rohangarg .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] dongjoon-hyun commented on pull request #11518: Upgrade ORC to 1.6.9

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #11518:
URL: https://github.com/apache/druid/pull/11518#issuecomment-890418361


   The one failure looks irrelevant to this one because the last commit (https://github.com/apache/druid/pull/11518/commits/2feb9a3c7246bffbc4c9e47b9f1ee926f7d3805d) is identical to the this commit (https://github.com/apache/druid/pull/11518/commits/9698b7d19227d2fe6388d9749a9a576cdeaa5b91) which passed all tested.
   ```
   [ERROR] Failures: 
   [ERROR]   ITKafkaIndexingServiceNonTransactionalParallelizedTest.testKafkaIndexDataWithWithAutoscaler:62->AbstractStreamIndexingTest.doTestIndexDataWithAutoscaler:342 ยป ISE
   [ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org