You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by zellerh <gi...@git.apache.org> on 2016/03/29 23:18:34 UTC
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
GitHub user zellerh opened a pull request:
https://github.com/apache/incubator-trafodion/pull/405
TRAFODION-1910 mxosrvr crashes on Hive query after reconnect
NATableDB is caching a pointer to a HiveClient_JNI object
(HiveMetaData::client_), but that object gets deallocated when a JDBC
client disconnects. Fixing this by keeping the HiveClient_JNI around
across sessions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zellerh/incubator-trafodion bug/1581
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/405.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #405
----
commit d97f9db39f0d35c1dc19566f5327f0812e521dc7
Author: Hans Zeller <hz...@apache.org>
Date: 2016-03-29T21:06:44Z
TRAFODION-1910 mxosrvr crashes on Hive query after reconnect
NATableDB is caching a pointer to a HiveClient_JNI object
(HiveMetaData::client_), but that object gets deallocated when a JDBC
client disconnects. Fixing this by keeping the HiveClient_JNI around
across sessions.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/405#discussion_r57908077
--- Diff: core/sql/cli/CliExtern.cpp ---
@@ -6316,7 +6316,8 @@ Lng32 SQL_EXEC_DeleteHbaseJNI()
threadContext->incrNumOfCliCalls();
HBaseClient_JNI::deleteInstance();
- HiveClient_JNI::deleteInstance();
+ // The Hive client persists across connections
+ // HiveClient_JNI::deleteInstance();
--- End diff --
Whenever a context is dropped, both hbaseClient_JNI and hiveClient_JNI is getting deleted. This CLI call is being called from the two places shown below. I am not sure about the need for this calls in these functions. If it is needed, I would think there would be similar problem with HBaseClient_JNI::deleteInstance too. Or is it better to get rid of this CLI call.
arkcmp/CmpStatement.cpp: SQL_EXEC_DeleteHbaseJNI();
sqlcomp/CmpSeabaseDDLcommon.cpp: SQL_EXEC_DeleteHbaseJNI();
In case of mxosrvr, there is default context. In case of T2 driver, the CliContext would be deallocated and these objects would be deleted. I am assuming that it should be ok to do that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/405#discussion_r57938814
--- Diff: core/sql/cli/CliExtern.cpp ---
@@ -6316,7 +6316,8 @@ Lng32 SQL_EXEC_DeleteHbaseJNI()
threadContext->incrNumOfCliCalls();
HBaseClient_JNI::deleteInstance();
- HiveClient_JNI::deleteInstance();
+ // The Hive client persists across connections
+ // HiveClient_JNI::deleteInstance();
--- End diff --
Sorry, I added a couple of comments to the JIRA, TRAFODION-1910 (https://issues.apache.org/jira/browse/TRAFODION-1910), should have added them here.
Let me try to summarize: The comments I made try to explain that we _could_ treat HiveClient_JNI and HBaseClient_JNI differently. Suresh is asking why we _should_ do that.
So, I think what you are telling me is that I should write a new fix that treats the two objects in a similar way. Hope to have that ready soon. Thanks for your input.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/405#discussion_r57803047
--- Diff: core/sql/cli/CliExtern.cpp ---
@@ -6316,7 +6316,8 @@ Lng32 SQL_EXEC_DeleteHbaseJNI()
threadContext->incrNumOfCliCalls();
HBaseClient_JNI::deleteInstance();
- HiveClient_JNI::deleteInstance();
+ // The Hive client persists across connections
+ // HiveClient_JNI::deleteInstance();
--- End diff --
Is there any security issue here? If we integrate with Hive security (and I don't know if we have or not) is there some notion of re-authentication at connection time?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/405#discussion_r57933613
--- Diff: core/sql/cli/CliExtern.cpp ---
@@ -6316,7 +6316,8 @@ Lng32 SQL_EXEC_DeleteHbaseJNI()
threadContext->incrNumOfCliCalls();
HBaseClient_JNI::deleteInstance();
- HiveClient_JNI::deleteInstance();
+ // The Hive client persists across connections
+ // HiveClient_JNI::deleteInstance();
--- End diff --
I would echo Selva's comment. Can you please explain how the HiveClient_JNI object is different from HBaseClient_JNI? I had tried to model HiveClient_JNI after HBaseClient_JNI and would like check if that assumption is incorrect in other places too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/405#discussion_r57803569
--- Diff: core/sql/cli/CliExtern.cpp ---
@@ -6316,7 +6316,8 @@ Lng32 SQL_EXEC_DeleteHbaseJNI()
threadContext->incrNumOfCliCalls();
HBaseClient_JNI::deleteInstance();
- HiveClient_JNI::deleteInstance();
+ // The Hive client persists across connections
+ // HiveClient_JNI::deleteInstance();
--- End diff --
Good question, and I think any security issues are (or need to be) handled in the HiveMetaData object, which already persists between connections. The object that now also persists is HiveClient_JNI (and the corresponding Java object). Those don't keep a cache or state about Hive metadata, they just have methods to read and validate Hive metadata. So, in short, no, I don't believe there is a security concern with this change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: TRAFODION-1910 mxosrvr crashes o...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/405
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---