Maybe I missed the discussion here, but seems to me that HiveServer2 can be configured with either SASL GSS (Kerberos) or SASL PLAIN (LDAP, CUSTOM username/password authentication), but not both simultaneously. Can I ask the reason for this, and whether it is straightforward to enable PLAIN and GSS simultaneously in the future? This is very useful for applications that have been supporting LDAP authentication on Hive, and when turn to Kerberos, legacy clients or non-kerberos clients would still be able to access kerberized HiveServer2.

Yu Gao
added a comment - 05/Aug/13 21:49 Maybe I missed the discussion here, but seems to me that HiveServer2 can be configured with either SASL GSS (Kerberos) or SASL PLAIN (LDAP, CUSTOM username/password authentication), but not both simultaneously. Can I ask the reason for this, and whether it is straightforward to enable PLAIN and GSS simultaneously in the future? This is very useful for applications that have been supporting LDAP authentication on Hive, and when turn to Kerberos, legacy clients or non-kerberos clients would still be able to access kerberized HiveServer2.
Thanks!

Navis While working on HS2 I encountered behavior indicating there were concurrency problems in the compiler. Since it takes much more time to execute a plan than to compile one, I figured it would be acceptable to serialize access to the compilation stage. I'll file a separate ticket to cover the task of fixing the concurrency bugs and removing the compiler lock.

Carl Steinbach
added a comment - 27/Mar/13 23:54 Navis While working on HS2 I encountered behavior indicating there were concurrency problems in the compiler. Since it takes much more time to execute a plan than to compile one, I figured it would be acceptable to serialize access to the compilation stage. I'll file a separate ticket to cover the task of fixing the concurrency bugs and removing the compiler lock.

After this patch was committed the tests have been taking an incredible amount of time. It looks like TestBeeLineDriver run the majority of the tests run by TestCliDriver again. Is this necessary, could a larger number of tests be excluded or the tests included be explicitly specified rather than the tests excluded?

Kevin Wilfong
added a comment - 13/Mar/13 19:54 After this patch was committed the tests have been taking an incredible amount of time. It looks like TestBeeLineDriver run the majority of the tests run by TestCliDriver again. Is this necessary, could a larger number of tests be excluded or the tests included be explicitly specified rather than the tests excluded?
It looks like the Jenkins tests may be timing out because of this as well. https://builds.apache.org/job/Hive-trunk-h0.21/2013/console
At the very least, could the ptest framework be fixed to parallelize these tests?

Ashutosh Chauhan
added a comment - 12/Mar/13 18:24 Committed to trunk. Did it in 4-checkins as Carl suggested. Hopefully, I got everything right while committing.
Thanks to Carl, Prasad, Thejas and others who contributed to this jumbo effort!

Thejas M Nair
added a comment - 08/Mar/13 01:05 HIVE-2935 -5.beeline.patch - Beeline,tests etc
Prasad Mujumdar This file does not have beeline test benchmark files. Are the test files from the old beelinepositive.tar.gz the ones that need to get checked in ?

Prasad Mujumdar
added a comment - 07/Mar/13 20:53 Ashutosh Chauhan The patch is rebased as of latest on trunk (commit c219de2d33820c1d66873283ec457a64f3aa4ea7). Its split into three patches as Carl suggested.

Carl Steinbach
added a comment - 07/Mar/13 18:51 @Ashutosh: Thanks for approving this. Since it's really big patch I wanted to suggest that we split it into three separate commits in order to make it easier for people to diff it in the future:
Code changes
Thrift generated code
Test outputs
Let me know if you want help with this. Thanks.

Thejas M Nair
added a comment - 23/Jan/13 18:42 HIVE-2935 .fix.unsecuredoAs.patch - an incremental patch to enable running of the query as the enduser. I can create a separate jira for this if necessary, after the main HS2 patch is in.

Thejas M Nair
added a comment - 10/Jan/13 20:17 Does the BigDecimal support requires the HIVE-2693 patch ?
Yes, but I think there has been good progress with the review there and hopefully gets committed soon. If not, we can move that to a different patch.
The last patch I updated did have thirft 0.9 generated bindings. Did you use that as the base for this work ?
The patch I uploaded was based on HIVE-2935 .2.notest.patch.txt. The thrift changes in your patch should work.

Thejas M Nair Thanks for the additional pieces! I will take a look at changes and let you know if I have any comments/suggestions.
Does the BigDecimal support requires the HIVE-2693 patch ? If that's the case then it's perhaps better to separate that out into a different patch. The last patch I updated did have thirft 0.9 generated bindings. Did you use that as the base for this work ?

Prasad Mujumdar
added a comment - 10/Jan/13 19:34 Thejas M Nair Thanks for the additional pieces! I will take a look at changes and let you know if I have any comments/suggestions.
Does the BigDecimal support requires the HIVE-2693 patch ? If that's the case then it's perhaps better to separate that out into a different patch. The last patch I updated did have thirft 0.9 generated bindings. Did you use that as the base for this work ?
Namit Jain and Ashutosh Chauhan would it be possible for you to take a look at HIVE-3785 ?

I am looking for suggestions on how to get this reviewed and committed. As the changes to core are being reviewed as part of HIVE-3785, I think it would be better to continue with earlier patch, and add the fixes in this patch separately.

The patch also has support for decimal type (HIVE-2693), which is yet to be committed.
The TestBeelineDriver tests work fine, if applied to branch 0.10. In trunk, changes to describe table output (HIVE-3140) is resulting in two headers getting printed and problems in formatting, and is causing TestBeelineDriver tests to fail. I think the change in HIVE-3140 needs to be revisited.

Highlight of the changes in this patch -

Adding support for decimal data type in HS2 and JDBC driver. Current implementation uses String to transport BigDecimal.

Incorporate basic unit test suite for hive server 2. A handpicked sample of TestCliDriver tests are run using jdbc + HS2 under TestBeelineDriver tests by default. These tests don't have issues described earlier and are expected to pass. It also limits the total test runtime.

Thejas M Nair
added a comment - 10/Jan/13 19:00 HIVE-2935 .3.patch.gz - Attaching patch with bug fixes and updates needed for recent changes in hive 0.10/trunk.
I am looking for suggestions on how to get this reviewed and committed. As the changes to core are being reviewed as part of HIVE-3785 , I think it would be better to continue with earlier patch, and add the fixes in this patch separately.
The patch also has support for decimal type ( HIVE-2693 ), which is yet to be committed.
The TestBeelineDriver tests work fine, if applied to branch 0.10. In trunk, changes to describe table output ( HIVE-3140 ) is resulting in two headers getting printed and problems in formatting, and is causing TestBeelineDriver tests to fail. I think the change in HIVE-3140 needs to be revisited.
Highlight of the changes in this patch -
Adding support for decimal data type in HS2 and JDBC driver. Current implementation uses String to transport BigDecimal.
Incorporate basic unit test suite for hive server 2. A handpicked sample of TestCliDriver tests are run using jdbc + HS2 under TestBeelineDriver tests by default. These tests don't have issues described earlier and are expected to pass. It also limits the total test runtime.
Fix problem with "current database" retained between sessions (in Hive class)
hiveserver2 with concurrency results in incorrect stats - disabling stats test, masking stats
regenerated thrift code for HiveServer2 + thrift 0.9
Re-enabling type verification for hive variables/settings
Fix OOM on the HiveServer - When running multiple execute operations within a statement, only the last one was being cleaned up at the server leaving leaving orphaned objects on the server.
Correct handling of binary column in server and driver.
ANSI standard dicates that null column should be printed as NULL - regen testbeeline benchmarks. Make it compatible with hive cli.
Enable doAs() functionality for HS2
Fixed string representation of complex type to bring jdbc driver in compliance with hive client.
add support for setting properties on cmdline (using -hiveconf) for hiveserver2
Making handleToSession map in SessionManager ConcurrentHashMap, as it is used concurrently by multiple threads.

Edward Capriolo
added a comment - 07/Jan/13 18:00 Yes you should not have two versions of thrift in your cp or stuff like this can happen even though the data may be wire compatible the libs usually are not.

Using CDH 4.1.2, which includes this patch. I think there's a problem with hive-jdbc which includes a JDBC driver for the two version of hiveserver.

For the first version of hiveserver, hive-jdbc-0.9.0-cdh4.1.2 depends on libthrift-1.5.0, which defines org.apache.thrift.TServiceClient as an Interface.

For hiveserver2, hive-jdbc-0.9.0-cdh4.1.2 depends on hive-service-0.9.0-cdh4.1.2, which depends on hive-service-0.9.0-cdh4.1.2. The later seems to include code from libthrift, and defines org.apache.thrift.TServiceClient as an abstract class.

Thus this happens:

java.lang.IncompatibleClassChangeError: class org.apache.hive.service.cli.thrift.TCLIService$Client has interface org.apache.thrift.TServiceClient as super class
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClassCond(Unknown Source)
at java.lang.ClassLoader.defineClass(Unknown Source)
at java.security.SecureClassLoader.defineClass(Unknown Source)
at java.net.URLClassLoader.defineClass(Unknown Source)
at java.net.URLClassLoader.access$000(Unknown Source)
at java.net.URLClassLoader$1.run(Unknown Source)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(Unknown Source)
at java.lang.ClassLoader.loadClass(Unknown Source)
at sun.misc.Launcher$AppClassLoader.loadClass(Unknown Source)
at java.lang.ClassLoader.loadClass(Unknown Source)
at org.apache.hive.jdbc.HiveConnection.openTransport(HiveConnection.java:157)
at org.apache.hive.jdbc.HiveConnection.<init>(HiveConnection.java:96)

Of course, I just have to remove libthrift from my libpath. But I just wanted to make Carl Steinbach know. (I used maven-dependency-plugin to get all dependent JARs, without thinking about what would be useless, or incompatible)

Nicolas Fouché
added a comment - 07/Jan/13 15:48 Using CDH 4.1.2, which includes this patch. I think there's a problem with hive-jdbc which includes a JDBC driver for the two version of hiveserver.
For the first version of hiveserver, hive-jdbc-0.9.0-cdh4.1.2 depends on libthrift-1.5.0, which defines org.apache.thrift.TServiceClient as an Interface.
For hiveserver2, hive-jdbc-0.9.0-cdh4.1.2 depends on hive-service-0.9.0-cdh4.1.2, which depends on hive-service-0.9.0-cdh4.1.2. The later seems to include code from libthrift, and defines org.apache.thrift.TServiceClient as an abstract class.
Thus this happens:
java.lang.IncompatibleClassChangeError: class org.apache.hive.service.cli.thrift.TCLIService$Client has interface org.apache.thrift.TServiceClient as super class
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClassCond(Unknown Source)
at java.lang.ClassLoader.defineClass(Unknown Source)
at java.security.SecureClassLoader.defineClass(Unknown Source)
at java.net.URLClassLoader.defineClass(Unknown Source)
at java.net.URLClassLoader.access$000(Unknown Source)
at java.net.URLClassLoader$1.run(Unknown Source)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(Unknown Source)
at java.lang.ClassLoader.loadClass(Unknown Source)
at sun.misc.Launcher$AppClassLoader.loadClass(Unknown Source)
at java.lang.ClassLoader.loadClass(Unknown Source)
at org.apache.hive.jdbc.HiveConnection.openTransport(HiveConnection.java:157)
at org.apache.hive.jdbc.HiveConnection.<init>(HiveConnection.java:96)
Of course, I just have to remove libthrift from my libpath. But I just wanted to make Carl Steinbach know. (I used maven-dependency-plugin to get all dependent JARs, without thinking about what would be useless, or incompatible)

I haven't looked at the patch either, and I agree with Ashutosh, that additive portions (like beeline) can be checked in.
However, would it be possible for you to break this patch, and extract the changes that you have made to the current hive code.
That presents a huge risk, and should be reviewed very carefully.

Or, you can take the other approach, which is to check in the new additive isolated components first (which will not be used), and
then have the code which touches the current hive code in a patch.

If someone is not using the hive server, what are the changes that this patch brings in ? That definitely needs to be reviewed very thoroughly.

Namit Jain
added a comment - 22/Nov/12 06:51 I haven't looked at the patch either, and I agree with Ashutosh, that additive portions (like beeline) can be checked in.
However, would it be possible for you to break this patch, and extract the changes that you have made to the current hive code.
That presents a huge risk, and should be reviewed very carefully.
Or, you can take the other approach, which is to check in the new additive isolated components first (which will not be used), and
then have the code which touches the current hive code in a patch.
If someone is not using the hive server, what are the changes that this patch brings in ? That definitely needs to be reviewed very thoroughly.

@Ashutosh: Sounds good to me. I'm in the processing of rebasing the patch, updating the test outputs, and addressing some small issues (e.g. the Python problem that Thaddeus found). I plan to get this posted by the end of the week. Maybe at that point you can give it a quick pass add +1 it if there are no red flags? Thanks.

Carl Steinbach
added a comment - 15/Nov/12 12:18 @Ashutosh: Sounds good to me. I'm in the processing of rebasing the patch, updating the test outputs, and addressing some small issues (e.g. the Python problem that Thaddeus found). I plan to get this posted by the end of the week. Maybe at that point you can give it a quick pass add +1 it if there are no red flags? Thanks.

I haven't reviewed this patch extensively, but looks like the codebase the patch (as it stands on jira) is introducing is nearly all additive. So it may be a good idea to get this patch in so folks can start playing with it.
Carl, do you think patch is ready to go in?

Ashutosh Chauhan
added a comment - 15/Nov/12 02:40 I haven't reviewed this patch extensively, but looks like the codebase the patch (as it stands on jira) is introducing is nearly all additive. So it may be a good idea to get this patch in so folks can start playing with it.
Carl, do you think patch is ready to go in?

There looks to be no way to derive the resultset metadata before query completion. This makes implementing SQLPrepare just about impossible. From our conversation last week at Strata, you were thinking to add a new call to support this. Is this still your plan?

George Chow
added a comment - 31/Oct/12 16:52 Hi Carl,
There looks to be no way to derive the resultset metadata before query completion. This makes implementing SQLPrepare just about impossible. From our conversation last week at Strata, you were thinking to add a new call to support this. Is this still your plan?
George

There is an issue with the patch in it's current state. If you make any program that references TCLIService.py or ttypes.py (i.e. a HiveServer2 python Thrift client), it will attempt to import the Thrift python bindings from your Thrift install as follows (first code line in the generated ttypes.py file):

from thrift.Thrift import *

In Thrift.py (as of version 0.7 through tip) there is a TType object defined. This protocol defines a second TType object, and the two get confused. The error that occurs when running the python program is:

This occurs because the python interpreter, when importing modules, ignores those that have already been imported and it thinks TType has already been imported. It then attempts to reference the local TType class, which has no I32 attribute. Changing the name of TType to TValueType in hive_service.thrift or something else that is more distinctly named solves this problem.

Thaddeus Diamond
added a comment - 28/Oct/12 23:37 There is an issue with the patch in it's current state. If you make any program that references TCLIService.py or ttypes.py (i.e. a HiveServer2 python Thrift client), it will attempt to import the Thrift python bindings from your Thrift install as follows (first code line in the generated ttypes.py file):
from thrift.Thrift import *
In Thrift.py (as of version 0.7 through tip) there is a TType object defined. This protocol defines a second TType object, and the two get confused. The error that occurs when running the python program is:
File "/home/mydir/hive/build/dist/lib/py/hive_service/ttypes.py", line 352, in TPrimitiveTypeEntry
(1, TType.I32, 'type', None, None, ), # 1
AttributeError: class TType has no attribute 'I32'
This occurs because the python interpreter, when importing modules, ignores those that have already been imported and it thinks TType has already been imported. It then attempts to reference the local TType class, which has no I32 attribute. Changing the name of TType to TValueType in hive_service.thrift or something else that is more distinctly named solves this problem.

We're not planning to combine these services. Rather, we're planning to run these services in parallel in the same JVM, but each service will operate on its own unique port. If you want to use the HS2 API and the metastore API at the same time you will need to create two separate connections and use two separate clients.

Carl Steinbach
added a comment - 17/Oct/12 07:36 We're not planning to combine these services. Rather, we're planning to run these services in parallel in the same JVM, but each service will operate on its own unique port. If you want to use the HS2 API and the metastore API at the same time you will need to create two separate connections and use two separate clients.

Thejas M Nair
added a comment - 17/Oct/12 02:46 we also designed HS2 with the intention that it would host a variety of services including the metastore thrift service (or some successor to that service)
Being able to host other services is great. But how do you plan to do it ? I could not find a way in thrift to combine services.

@Thejas: Not extending the Hive metastore Thrift API was a conscious design decision on our part. We think the current metastore Thrift API suffers from a variety of design defects, and we're opposed to burdening the new HS2 Thrift API with that technical debt. On the other hand, we also designed HS2 with the intention that it would host a variety of services including the metastore thrift service (or some successor to that service), and we plan to add support for this in a followup patch.

Carl Steinbach
added a comment - 17/Oct/12 02:36 @Thejas: Not extending the Hive metastore Thrift API was a conscious design decision on our part. We think the current metastore Thrift API suffers from a variety of design defects, and we're opposed to burdening the new HS2 Thrift API with that technical debt. On the other hand, we also designed HS2 with the intention that it would host a variety of services including the metastore thrift service (or some successor to that service), and we plan to add support for this in a followup patch.

Carl,
The existing HiveServer also supports the MetaStore api's (the HiveServer thrift service inherits MetaStore thrift service). This means that we need to only one server for both. In case of this patch, the server does not include MetaStore service . I think we should have the HiveServer2 thrift service inherit metastore service as well. This will avoid increasing the complexity of managing clusters where you want both interfaces to be supported.

Thejas M Nair
added a comment - 16/Oct/12 22:35 Carl,
The existing HiveServer also supports the MetaStore api's (the HiveServer thrift service inherits MetaStore thrift service). This means that we need to only one server for both. In case of this patch, the server does not include MetaStore service . I think we should have the HiveServer2 thrift service inherit metastore service as well. This will avoid increasing the complexity of managing clusters where you want both interfaces to be supported.

I think this patch is in pretty good shape right now. Since this work is almsot completely decoupled from the rest of Hive I think the best option would be to commit it directly to trunk and address any bugs in separate tickets. However, in an earlier email to the PMC mailing list you argued that the size of this patch precludes the possibility of reviewing it, so I am in the process of breaking it into smaller pieces (as detailed above) and will start posting those pieces for review over the next couple of days. I don't think committing this to a separate branch will make the code any easier to review, and there are no established conventions within this project concerning development work on feature branches. I would prefer that we not make this patch the testcase for a new policy.

Carl: I'm fine with checking directly into trunk and working from there. My goal isn't to create a branch but to figure out how to collaborate. It seems like people are starting to reviewing it.

Alan Gates
added a comment - 16/Oct/12 17:06 I think this patch is in pretty good shape right now. Since this work is almsot completely decoupled from the rest of Hive I think the best option would be to commit it directly to trunk and address any bugs in separate tickets. However, in an earlier email to the PMC mailing list you argued that the size of this patch precludes the possibility of reviewing it, so I am in the process of breaking it into smaller pieces (as detailed above) and will start posting those pieces for review over the next couple of days. I don't think committing this to a separate branch will make the code any easier to review, and there are no established conventions within this project concerning development work on feature branches. I would prefer that we not make this patch the testcase for a new policy.
Carl: I'm fine with checking directly into trunk and working from there. My goal isn't to create a branch but to figure out how to collaborate. It seems like people are starting to reviewing it.

@Thejas: Thanks for the report about running the tests concurrently. I will try reproducing this on my end tonight and see if I get similar results.

1. metadata such as numRows and rawDataSize get reported as 0 .

I think there's a good chance that the table stats subsystem is not thread-safe. I'm inclined to disable these tests and address this issue as a followup. Please let me know if you disagree with this approach.

Carl Steinbach
added a comment - 16/Oct/12 00:49 @Thejas: Thanks for the report about running the tests concurrently. I will try reproducing this on my end tonight and see if I get similar results.
1. metadata such as numRows and rawDataSize get reported as 0 .
I think there's a good chance that the table stats subsystem is not thread-safe. I'm inclined to disable these tests and address this issue as a followup. Please let me know if you disagree with this approach.
2. DDL command failed
Haven't seen this before. Let me see if I can reproduce it.

If we start posting patches on top of the existing patches we'll have a mess.

Please don't do this. The convention in this project (as well as every other Apache project that I am familiar with) is to post review comments and give the original author time to respond. So far I have been busy splitting this patch into smaller pieces in order to satisfy your previous request. However, I'm more than willing to post a review request for the patch as it stands if that would expedite the review process. It's up to you. Please let me know how you would like proceed.

Carl Steinbach
added a comment - 16/Oct/12 00:26 @Alan:
If we start posting patches on top of the existing patches we'll have a mess.
Please don't do this. The convention in this project (as well as every other Apache project that I am familiar with) is to post review comments and give the original author time to respond. So far I have been busy splitting this patch into smaller pieces in order to satisfy your previous request. However, I'm more than willing to post a review request for the patch as it stands if that would expedite the review process. It's up to you. Please let me know how you would like proceed.

We attempted to make the diff masking logic in QFileClient far more selective about what it elides than the current logic found in QTestUtil. For example, QTestUtil currently masks the entire output of the DESCRIBE EXTENDED command. One of the unintended consequences of this effort is that we appear to have unmasked some non-deterministic behavior in the output of these commands. For example, I just ran alter_merge_stats.q and observed that it fails because the order of several table parameters in the output of DESCRIBE EXTENDED is reversed. The other tests you listed pass when I run them, but I'm guessing that they would eventually fail for the same reason. I will file a subtask to cover fixing this.

Carl Steinbach
added a comment - 16/Oct/12 00:19
1. metadata such as numRows and rawDataSize get reported as 0 .
eg -
alter_merge_stats
bucketmapjoin2.q
stats18.q - desc formatted stats_part partition query - missing some stats records in output
union22 - stats records missing
We attempted to make the diff masking logic in QFileClient far more selective about what it elides than the current logic found in QTestUtil. For example, QTestUtil currently masks the entire output of the DESCRIBE EXTENDED command. One of the unintended consequences of this effort is that we appear to have unmasked some non-deterministic behavior in the output of these commands. For example, I just ran alter_merge_stats.q and observed that it fails because the order of several table parameters in the output of DESCRIBE EXTENDED is reversed. The other tests you listed pass when I run them, but I'm guessing that they would eventually fail for the same reason. I will file a subtask to cover fixing this.

@Thejas: One of the problems we encountered with running qfile tests in parallel is that many of these tests create temporary tables and indexes, and several also modify the underlying default source tables. Both of these issues affect the output of the tests, and when you add in concurrency you end up with non-deterministic output that is impossible to validate with the diff-based verification scheme we have in place today. QFileClient works around this problem by running each qfile test in its own DB/Schema. This solves the problem for the overwhelming majority of qfile tests that exist in Hive today. However, there are several notable drawbacks:

We can't support tests that create new DB/Schemas, use the 'USE' command to switch databases, create catalog objects in separate DB/Schemas using fully qualified names, invoke the 'SHOW DATABASES' command, etc. Tests which fall into this category include ctas_uses_database_location.q, add_part_exist.q, etc. I added most of these tests to the test.beeline.positive.exclude list in build.properties, but clearly I missed some, and it also looks like there are some tests in that list that need to be removed. I'll update this soon.

Index tests such as alter_index.q are also affected since the full name of an index catalog object is $
{table_schema}

__$

{table_name}

_$

{index_name}

__. We should be able to work around this specific problem by defining a substitution property for each test corresponding to the db name. I will file a separate subtask for this issue.

This partitioning scheme allows us to test concurrent DDL/DML commands in separate namespaces, but doesn't provide any coverage for running concurrent DDL/DML in the same namespace. I don't think it's feasible to do this with the current set of qfiles, and propose instead that we create a separate test that concurrently runs a carefully selected subset of these qfiles in the same namespace.

Most of the tests that you listed above fall into one of these categories. Please let me know if you find any that don't and I'll look at them more closely.

A separate but related matter is that if we commit this patch with TestBeeLineDriver enabled, the overall time to run all tests will roughly double from ~4hrs to ~8hrs. My preference is do deprecate TestCliDriver in favor of TestBeeLineDriver, but I can't make that decision on my own.

Carl Steinbach
added a comment - 15/Oct/12 23:59 @Thejas: One of the problems we encountered with running qfile tests in parallel is that many of these tests create temporary tables and indexes, and several also modify the underlying default source tables. Both of these issues affect the output of the tests, and when you add in concurrency you end up with non-deterministic output that is impossible to validate with the diff-based verification scheme we have in place today. QFileClient works around this problem by running each qfile test in its own DB/Schema. This solves the problem for the overwhelming majority of qfile tests that exist in Hive today. However, there are several notable drawbacks:
We can't support tests that create new DB/Schemas, use the 'USE' command to switch databases, create catalog objects in separate DB/Schemas using fully qualified names, invoke the 'SHOW DATABASES' command, etc. Tests which fall into this category include ctas_uses_database_location.q, add_part_exist.q, etc. I added most of these tests to the test.beeline.positive.exclude list in build.properties, but clearly I missed some, and it also looks like there are some tests in that list that need to be removed. I'll update this soon.
Index tests such as alter_index.q are also affected since the full name of an index catalog object is $
{table_schema}
__$
{table_name}
_$
{index_name}
__. We should be able to work around this specific problem by defining a substitution property for each test corresponding to the db name. I will file a separate subtask for this issue.
This partitioning scheme allows us to test concurrent DDL/DML commands in separate namespaces, but doesn't provide any coverage for running concurrent DDL/DML in the same namespace. I don't think it's feasible to do this with the current set of qfiles, and propose instead that we create a separate test that concurrently runs a carefully selected subset of these qfiles in the same namespace.
Most of the tests that you listed above fall into one of these categories. Please let me know if you find any that don't and I'll look at them more closely.
A separate but related matter is that if we commit this patch with TestBeeLineDriver enabled, the overall time to run all tests will roughly double from ~4hrs to ~8hrs. My preference is do deprecate TestCliDriver in favor of TestBeeLineDriver, but I can't make that decision on my own.

With concurrency=10, we see around 25 additional failures. We ran it twice with concurrency=10 and once with concurrency=100, the total number of failures happened to be same - 70, but some of the tests that failed were unique in each run. The additional tests that failed in one of the test runs with concurrency=10 are -
alter3
alter_merge_stats
bucketmapjoin2
create_escape
groupby_sort_1
groupby_sort_skew_1
input_part10
join_filters_overlap
join_map_ppr
load_fs
pcr
ppd_union_view
rcfile_default_format
sample10
sort_merge_join_desc_5
sort_merge_join_desc_6
sort_merge_join_desc_7
stats0
stats11
stats18
stats_empty_dyn_part
stats_empty_partition
union22
union24
updateAccessTime

I went through a sample of the failures, and most of them have two types of failures -
1. metadata such as numRows and rawDataSize get reported as 0 .
eg -
alter_merge_stats
bucketmapjoin2.q
stats18.q - desc formatted stats_part partition query - missing some stats records in output
union22 - stats records missing

groupby_sort_1.q - difference in '.. rows selected' that is printed after explain .

Thejas M Nair
added a comment - 15/Oct/12 23:23 With concurrency=10, we see around 25 additional failures. We ran it twice with concurrency=10 and once with concurrency=100, the total number of failures happened to be same - 70, but some of the tests that failed were unique in each run. The additional tests that failed in one of the test runs with concurrency=10 are -
alter3
alter_merge_stats
bucketmapjoin2
create_escape
groupby_sort_1
groupby_sort_skew_1
input_part10
join_filters_overlap
join_map_ppr
load_fs
pcr
ppd_union_view
rcfile_default_format
sample10
sort_merge_join_desc_5
sort_merge_join_desc_6
sort_merge_join_desc_7
stats0
stats11
stats18
stats_empty_dyn_part
stats_empty_partition
union22
union24
updateAccessTime
The tests have been run with HIVE-2935 .2.nothrift.patch.txt .
I went through a sample of the failures, and most of them have two types of failures -
1. metadata such as numRows and rawDataSize get reported as 0 .
eg -
alter_merge_stats
bucketmapjoin2.q
stats18.q - desc formatted stats_part partition query - missing some stats records in output
union22 - stats records missing
groupby_sort_1.q - difference in '.. rows selected' that is printed after explain .
2. DDL command failed
example -
Error: Error while processing statement: FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask (state=08S01,code=1)
Error: Error while processing statement: FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask (state=08S01,code=1)
Aborting command set because "force" is false and command failed: "drop table alter_partition_format_test;"
Aborting command set because "force" is false and command failed: "!run !!
{qFileDirectory}
!!/alter_partition_format_loc.q"
tests-
rcfile_createas1.q.out
alter_partition_format_loc.q.out

Thejas M Nair
added a comment - 15/Oct/12 22:42 Carl,
There are some golden files missing in beeleinepositive.tar.gz, and some tests outputs have difference of white space with the golden files.
Tests with missing files -
alter_index.q
ctas_uses_database_location.q.out
filter_join_breaktask2.q.out
index_creation.q.out
input9.q.out
insert1.q.out
metadata_export_drop.q.out
recursive_dir.q.out
rename_external_partition_location.q.out
reset_conf.q.out
smb_mapjoin_11.q.out
table_access_keys_stats.q.out
I saw a total of 45 failures, when I ran with concurrency=1 .
Some of the other failures, (non white space diff) that I analyzed (and still have notes for) -
inputddl5.q.out (unicode vs ??)
bucketmapjoin7.q.out - query in test not guarenteed to produce same results

Alan and I have been talking about various approaches to JDBC / ODBC. We'll work to get something on the list in the next few weeks. We've got a RESTful API to HCat and Hive execution prototyped (with help from a bunch of folks). It seems like we can put a thin client together for ODBC & JDBC based on these APIs that will be more consistent with future plans for adding web services to YARN and current work on HCat.

@Alan: Are you guys still working on this? If so are there any design docs I can look at? The RESTful HCat API for Hive looks like it's batch oriented. I don't think it's going to be possible to build an ODBC or JDBC driver on top of that API without adding explicit support for sessions and statement oriented execute/fetch calls.

Carl Steinbach
added a comment - 15/Oct/12 22:25 Alan and I have been talking about various approaches to JDBC / ODBC. We'll work to get something on the list in the next few weeks. We've got a RESTful API to HCat and Hive execution prototyped (with help from a bunch of folks). It seems like we can put a thin client together for ODBC & JDBC based on these APIs that will be more consistent with future plans for adding web services to YARN and current work on HCat.
@Alan: Are you guys still working on this? If so are there any design docs I can look at? The RESTful HCat API for Hive looks like it's batch oriented. I don't think it's going to be possible to build an ODBC or JDBC driver on top of that API without adding explicit support for sessions and statement oriented execute/fetch calls.

@Alan: I think this patch is in pretty good shape right now. Since this work is almsot completely decoupled from the rest of Hive I think the best option would be to commit it directly to trunk and address any bugs in separate tickets. However, in an earlier email to the PMC mailing list you argued that the size of this patch precludes the possibility of reviewing it, so I am in the process of breaking it into smaller pieces (as detailed above) and will start posting those pieces for review over the next couple of days. I don't think committing this to a separate branch will make the code any easier to review, and there are no established conventions within this project concerning development work on feature branches. I would prefer that we not make this patch the testcase for a new policy.

Carl Steinbach
added a comment - 15/Oct/12 22:18 @Alan: I think this patch is in pretty good shape right now. Since this work is almsot completely decoupled from the rest of Hive I think the best option would be to commit it directly to trunk and address any bugs in separate tickets. However, in an earlier email to the PMC mailing list you argued that the size of this patch precludes the possibility of reviewing it, so I am in the process of breaking it into smaller pieces (as detailed above) and will start posting those pieces for review over the next couple of days. I don't think committing this to a separate branch will make the code any easier to review, and there are no established conventions within this project concerning development work on feature branches. I would prefer that we not make this patch the testcase for a new policy.

Carl, we'd like to start helping get these patches in shape for commit to the trunk. If we start posting patches on top of the existing patches we'll have a mess. Does it make sense to commit these quickly to a branch so we can collaborate and then merge them into trunk when they're solid?

Alan Gates
added a comment - 13/Oct/12 01:12 Carl, we'd like to start helping get these patches in shape for commit to the trunk. If we start posting patches on top of the existing patches we'll have a mess. Does it make sense to commit these quickly to a branch so we can collaborate and then merge them into trunk when they're solid?

@Carl: One of the concern which folks have with hive server (HS1) is how well it can handle concurrency. Can you explain how that concern is taken care of in HS2? In the course of development and testing of HS2 did you discover any concurrency bugs? It will be good to spell those concurrency bugs out and fixes that you made to solve it. Same goes for memory leaks, socket leaks, file descriptor leaks etc., the typical resource leaks which occurs in long running server processes. Was there any work and/or testing in those areas ? Are those tests reproducible?

Ashutosh Chauhan
added a comment - 06/Oct/12 00:05 @Carl: One of the concern which folks have with hive server (HS1) is how well it can handle concurrency. Can you explain how that concern is taken care of in HS2? In the course of development and testing of HS2 did you discover any concurrency bugs? It will be good to spell those concurrency bugs out and fixes that you made to solve it. Same goes for memory leaks, socket leaks, file descriptor leaks etc., the typical resource leaks which occurs in long running server processes. Was there any work and/or testing in those areas ? Are those tests reproducible?

To follow on Edward's comment, I don't understand why beeline is in the patch. Is it integral to HiveServer2?

People need a way to interact with HiveServer2. We could have spent time modifying the existing CLI to work with HS2, but we decided against this approach because a) the HiveCLI has a lot of bugs, and b) we risked introducing new bugs in the process of modifying the CLI to work with both HS1 and HS2. We included BeeLine in this patch because most of the test coverage we have provided for HiveServer2 depends on the new TestBeeLineDriver, which in turn depends on BeeLine.

Carl Steinbach
added a comment - 05/Oct/12 21:54 To follow on Edward's comment, I don't understand why beeline is in the patch. Is it integral to HiveServer2?
People need a way to interact with HiveServer2. We could have spent time modifying the existing CLI to work with HS2, but we decided against this approach because a) the HiveCLI has a lot of bugs, and b) we risked introducing new bugs in the process of modifying the CLI to work with both HS1 and HS2. We included BeeLine in this patch because most of the test coverage we have provided for HiveServer2 depends on the new TestBeeLineDriver, which in turn depends on BeeLine.

What do you think about fork beeline as a separate project inside hive. Suchs as hive-beeline. Because a majority of this patch looks to be beeline with some subtle tweeks.

In the current version of the patch the BeeLine code is included in the hive-cli package. I think this makes sense since BeeLine is a CLI. On the other hand, if we added a new package for beeline we would be able to avoid adding dependencies on the other Hive JARs that the current CLI mandates we include. Providing this separation will probably be beneficial in the long term so I'll start making the change and will submit this in another ticket.

Carl Steinbach
added a comment - 05/Oct/12 21:47 What do you think about fork beeline as a separate project inside hive. Suchs as hive-beeline. Because a majority of this patch looks to be beeline with some subtle tweeks.
In the current version of the patch the BeeLine code is included in the hive-cli package. I think this makes sense since BeeLine is a CLI. On the other hand, if we added a new package for beeline we would be able to avoid adding dependencies on the other Hive JARs that the current CLI mandates we include. Providing this separation will probably be beneficial in the long term so I'll start making the change and will submit this in another ticket.

@Namit: The only change we made to the Driver class was to wrap a monitor lock around the compile() call in order to serialize access to the compilation phase. I can split this out into a separate patch if you think that would helpful.

Carl Steinbach
added a comment - 05/Oct/12 21:23 @Namit: The only change we made to the Driver class was to wrap a monitor lock around the compile() call in order to serialize access to the compilation phase. I can split this out into a separate patch if you think that would helpful.

Edward Capriolo
added a comment - 05/Oct/12 14:40 What do you think about fork beeline as a separate project inside hive. Suchs as hive-beeline. Because a majority of this patch looks to be beeline with some subtle tweeks.

Namit Jain
added a comment - 05/Oct/12 12:54 It would be useful if you can split the hive driver changes (if any) in a different patch.
I mean, the parts of the patch that possibly affect the stability of current hive (not using hive server).

The attached patches contain a complete, working version of HiveServer2. Before going into details about the contents of the patch I want to first quickly review how to apply the patch and try out the new server:

This a JDBC CLI for Hive based on the SQLLine CLI. An earlier version of BeeLine which had a dependency on SQLLine was previously added in HIVE-3100. However, while working on HS2 we discovered bugs in SQLLine that we needed to fix, and also needed to build in some extensions in order to support the BeeLine test driver. Adding the code directly to Hive seemed like the best option since the upstream project is no longer actively maintained.

cli/src/java/org/apache/hive/cli/beeline/util/QFileClient.java

This is a beeline test client used by TestBeeLineDriver. This class also provides an improved version of the output masking functionality currently located in QTestUtil.

common/src/java/org/apache/hive/common/util/*.java

Utility classes borrowed from Hadoop.

data/files/types/primitives/*

Data files for a 'primitives' table that contains all Hive primitive types along with NULLs.

data/scripts/q_test_*.sql

Test initialization scripts that are used to create and initialize all of the tables that are referenced by CliDriver tests. Called from QFileClient.

jdbc/src/java/org/apache/hive/jdbc/*

The HS2 JDBC driver.

service/if/cli_service.thrift

The CliService Thrift IDL file.

service/src/java/org/apache/hive/service/*

Service infrastructure classes borrowed from o.a.hadoop.yarn.service

service/src/java/org/apache/hive/service/auth/*

Kerberos/LDAP/SASL auth code for HS2

service/src/java/org/apache/hive/service/cli/*

Implementation classes for CLIService. These classes form the core of HiveServer2.

service/src/java/org/apache/hive/service/server/HiveServer2.java

HiveServer2 class. At the moment it just starts the CLIService, but we plan to extend it with other pluggable services in the future.

{{testutils/junit/*}

Utility classes for running concurrent JUnit tests. Most of this code was borrowed from tempus-fugit.

This code was a collaborative effort between me and my colleague Prasad Mujumdar. Prasad is wholly responsible for the new JDBC driver and authorization code. We worked together on the other parts.

We plan to break this monolithic patch up into several smaller patches in order to make the review process easier. Here's our initial plan for how to do this:

Review/commit the beeline CLI (note that this can be used with the existing JDBC driver and HiveServer1)

Review/commit the BeeLine test driver and new test outputs

Review/commit the HiveServer2 core component.

Review/commit the HiveServer2 JDBC driver.

We would appreciate receiving feedback from the Hive committers about whether or not this plan makes sense.

Carl Steinbach
added a comment - 04/Oct/12 18:25 The attached patches contain a complete, working version of HiveServer2. Before going into details about the contents of the patch I want to first quickly review how to apply the patch and try out the new server:
1) Download and apply HIVE-2935 .2.nothrift.patch.txt
2) Run the Thrift code generator (make sure $THRIFT_HOME points to thrift version 0.7.0)
% ant thriftif -Dthrift.home=$THRIFT_HOME
3) Optionally download and unpack the beeline test outputs in the ql/src/test/results directory.
4) Build Hive:
% ant clean package
5) Start HiveServer2
% hiveserver2
6) From another window start the beeline CLI and connect to HiveServer2:
% beeline
Hive version 0.10.0-SNAPSHOT by Apache
beeline> !connect jdbc:hive2://localhost:10000 scott tiger org.apache.hive.jdbc.HiveDriver
!connect jdbc:hive2://localhost:10000 scott tiger org.apache.hive.jdbc.HiveDriver
Connecting to jdbc:hive2://localhost:10000
Connected to: Hive (version 0.10.0)
Driver: Hive (version 0.10.0-SNAPSHOT)
Transaction isolation: TRANSACTION_REPEATABLE_READ
0: jdbc:hive2://localhost:10000> show tables;
show tables;
+-------------------+
| tab_name |
+-------------------+
| primitives |
| src |
| src1 |
| src_json |
| src_sequencefile |
| src_thrift |
| srcbucket |
| srcbucket2 |
| srcpart |
+-------------------+
9 rows selected (1.079 seconds)
If you downloaded the test outputs you can also try running the new BeeLineTestDriver:
% ant test -Dtestcase=TestBeeLineDriver -Dtest.concurrency.num.threads=10
Patch contents:
cli/src/java/org/apache/hive/cli/beeline/*.java
This a JDBC CLI for Hive based on the SQLLine CLI. An earlier version of BeeLine which had a dependency on SQLLine was previously added in HIVE-3100 . However, while working on HS2 we discovered bugs in SQLLine that we needed to fix, and also needed to build in some extensions in order to support the BeeLine test driver. Adding the code directly to Hive seemed like the best option since the upstream project is no longer actively maintained.
cli/src/java/org/apache/hive/cli/beeline/util/QFileClient.java
This is a beeline test client used by TestBeeLineDriver. This class also provides an improved version of the output masking functionality currently located in QTestUtil.
common/src/java/org/apache/hive/common/util/*.java
Utility classes borrowed from Hadoop.
data/files/types/primitives/*
Data files for a 'primitives' table that contains all Hive primitive types along with NULLs.
data/scripts/q_test_*.sql
Test initialization scripts that are used to create and initialize all of the tables that are referenced by CliDriver tests. Called from QFileClient.
jdbc/src/java/org/apache/hive/jdbc/*
The HS2 JDBC driver.
service/if/cli_service.thrift
The CliService Thrift IDL file.
service/src/java/org/apache/hive/service/*
Service infrastructure classes borrowed from o.a.hadoop.yarn.service
service/src/java/org/apache/hive/service/auth/*
Kerberos/LDAP/SASL auth code for HS2
service/src/java/org/apache/hive/service/cli/*
Implementation classes for CLIService. These classes form the core of HiveServer2.
service/src/java/org/apache/hive/service/server/HiveServer2.java
HiveServer2 class. At the moment it just starts the CLIService, but we plan to extend it with other pluggable services in the future.
{{testutils/junit/*}
Utility classes for running concurrent JUnit tests. Most of this code was borrowed from tempus-fugit.
This code was a collaborative effort between me and my colleague Prasad Mujumdar. Prasad is wholly responsible for the new JDBC driver and authorization code. We worked together on the other parts.
We plan to break this monolithic patch up into several smaller patches in order to make the review process easier. Here's our initial plan for how to do this:
Review/commit the beeline CLI (note that this can be used with the existing JDBC driver and HiveServer1)
Review/commit the BeeLine test driver and new test outputs
Review/commit the HiveServer2 core component.
Review/commit the HiveServer2 JDBC driver.
We would appreciate receiving feedback from the Hive committers about whether or not this plan makes sense.
Thanks!

Alan and I have been talking about various approaches to JDBC / ODBC. We'll work to get something on the list in the next few weeks. We've got a RESTful API to HCat and Hive execution prototyped (with help from a bunch of folks). It seems like we can put a thin client together for ODBC & JDBC based on these APIs that will be more consistent with future plans for adding web services to YARN and current work on HCat.

eric baldeschwieler
added a comment - 05/Jun/12 07:38 Alan and I have been talking about various approaches to JDBC / ODBC. We'll work to get something on the list in the next few weeks. We've got a RESTful API to HCat and Hive execution prototyped (with help from a bunch of folks). It seems like we can put a thin client together for ODBC & JDBC based on these APIs that will be more consistent with future plans for adding web services to YARN and current work on HCat.

It should also be possible for Hive Server to use HCatalog as metastore [instead of embedded] and Hive Server should only execute queries. I dont think thats possible with the current implementation [please correct me if I am wrong].

Thiruvel Thirumoolan
added a comment - 04/Jun/12 19:01 It should also be possible for Hive Server to use HCatalog as metastore [instead of embedded] and Hive Server should only execute queries. I dont think thats possible with the current implementation [please correct me if I am wrong] .

Have we also considered going through the hive source code and removing and static thread local type objects pass a context around. I feel hive server and other such code would naturally have less concurrency problems if we cleaned up thread local through the code.

Edward Capriolo
added a comment - 28/Apr/12 16:09 Have we also considered going through the hive source code and removing and static thread local type objects pass a context around. I feel hive server and other such code would naturally have less concurrency problems if we cleaned up thread local through the code.

Carl Steinbach
added a comment - 09/Apr/12 20:04 This is an umbrella JIRA for the HiveServer2 work.
A proposal for the HiveServer2 Thrift API is available here:
https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Thrift+API