Vihang Karajgaonkar 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 12: (11 comments) Addressed review comments 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: > Is this flakiness specific to the hive 3 config? Or the races were already I checked and confirmed that the notification listener is working is generating the events as expected but there may be more to it. I will dig more. Even if there is a some problem with the event data, these races are still present can show up when Sentry is slow for any reason to update its entries. http://gerrit.cloudera.org:8080/#/c/13005/9//COMMIT_MSG@65 PS9, Line 65: > maybe we shoudl disable these tests when running with hive 3 since we don't So far, I would expect these tests work without any modifications on hive-2 builds (results pending for the last job I triggered). When we turn on cdp for jobs we should re-investigate how to fix these tests. http://gerrit.cloudera.org:8080/#/c/13005/3/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/13005/3/bin/impala-config.sh@167 PS3, Line 167: export IMPALA_RANGER_VERSION=1.2.0.6.0.99.0-45 > This number looks like a CDH_BUILD_NUMBER, and is probably from the same na I was not aware of this.. Most of the pending patches are merged into official builds so we may not need this anymore. http://gerrit.cloudera.org:8080/#/c/13005/3/bin/impala-config.sh@199 PS3, Line 199: # TODO(todd) switch to an official build. > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/13005/3/bin/impala-config.sh@203 PS3, Line 203: # CDH hive version is used to build and deploy in minicluster when USE_CDP_HIVE is > line too long (92 > 90) Done http://gerrit.cloudera.org:8080/#/c/13005/3/bin/impala-config.sh@212 PS3, Line 212: fi > line too long (106 > 90) Done 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 You are right, this is not necessary. removed it. 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: throws MetaException { : return null; : } : }; : : /** > Are these changes actually used right now? I think this stuff ended up bein Yeah. reverted the changes to this file in the latest patch 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: private static MetastoreEventsProcessor instance; > Can you explain what's going on with this part of the change? Did Sentry mo Sentry did not move to the JSONMessageFactory (and I believe this may be the reason why the test_ownership.py fails). However, in the shims approach we use the ExtendedJsonFactory for hive-2 builds and in hive-3 build we don't expect Sentry to be there. I will keep investigating while I debug cdp jobs. 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="-Xdebug -Xrunjdwp:transport=dt_socket,server=y,\ > probably remove this Done http://gerrit.cloudera.org:8080/#/c/13005/9/testdata/bin/run-hive-server.sh@93 PS9, Line 93: if [ ${ONLY_METASTORE} -eq 0 ]; then > this is in another patch- guess we can rebase this on top of that one to pi Rebased my patch. -- 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: 12 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: Mon, 29 Apr 2019 20:59:24 +0000 Gerrit-HasComments: Yes
