Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13005 )
Change subject: IMPALA-8369 : Impala should be able to interoperate with Hive 3.1.0 ...................................................................... Patch Set 9: (7 comments) Took a look through this. I'll see if I can get some time tomorrow to try out a "shims" approach as well for comparison. It looks like the majority of the changes here, though, are pretty innocuous (eg copying some stuff into our code where we were relying on hive-internal stuff) and could be made either way. http://gerrit.cloudera.org:8080/#/c/13005/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13005/9//COMMIT_MSG@58 PS9, Line 58: 4. The Sentry object ownership tests are flaky. There are Is this flakiness specific to the hive 3 config? Or the races were already there? Is the problem here that we don't have the notification listener working properly so we're relying on some periodic sync instead of notifications? http://gerrit.cloudera.org:8080/#/c/13005/9//COMMIT_MSG@65 PS9, Line 65: work-around these issues. maybe we shoudl disable these tests when running with hive 3 since we don't anticipate using Sentry with Hive 3 anyway http://gerrit.cloudera.org:8080/#/c/13005/9/bin/set-classpath.sh File bin/set-classpath.sh: http://gerrit.cloudera.org:8080/#/c/13005/9/bin/set-classpath.sh@30 PS9, Line 30: "$IMPALA_HOME"/shaded-deps/target/impala-shaded-deps-0.1-SNAPSHOT.jar:\ why is this necessary? shouldn't the shaded-deps dependency also end up in target/dependency/ like the others handled on line 35? http://gerrit.cloudera.org:8080/#/c/13005/9/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java: http://gerrit.cloudera.org:8080/#/c/13005/9/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@68 PS9, Line 68: private List<Integer> serverVersion = null; : : private static final Ordering<Iterable<Integer>> VERSION_ORDERING = : Ordering.<Integer>natural().lexicographical().nullsLast(); : private static final ImmutableList<Integer> CATALOGS_VERSION = : ImmutableList.of(3, 0, 0); Are these changes actually used right now? I think this stuff ended up being in the Hive impl rather than being doable in this wrapper http://gerrit.cloudera.org:8080/#/c/13005/9/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/13005/9/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@173 PS9, Line 173: MessageFactory.getDefaultInstance(new HiveConf()).getDeserializer(); Can you explain what's going on with this part of the change? Did Sentry move to the normal JSONMessageFactory? http://gerrit.cloudera.org:8080/#/c/13005/9/testdata/bin/run-hive-server.sh File testdata/bin/run-hive-server.sh: http://gerrit.cloudera.org:8080/#/c/13005/9/testdata/bin/run-hive-server.sh@66 PS9, Line 66: export HIVE_METASTORE_HADOOP_OPTS="-verbose:class -Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=30010" probably remove this http://gerrit.cloudera.org:8080/#/c/13005/9/testdata/bin/run-hive-server.sh@93 PS9, Line 93: # For Hive 3, we use Tez for execution. We have to add it to the HS2 classpath. this is in another patch- guess we can rebase this on top of that one to pick it up -- To view, visit http://gerrit.cloudera.org:8080/13005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45a4dadbdfe30a02f722dbd917a49bc182fc6436 Gerrit-Change-Number: 13005 Gerrit-PatchSet: 9 Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Sudhanshu Arora <sudhan...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 26 Apr 2019 06:51:52 +0000 Gerrit-HasComments: Yes
