Re: Review Request 72403: HIVE-19064

2020-04-21 Thread Krisztian Kasa

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72403/
---

(Updated April 22, 2020, 5:22 a.m.)


Review request for hive and Jesús Camacho Rodríguez.


Bugs: HIVE-19064
https://issues.apache.org/jira/browse/HIVE-19064


Repository: hive-git


Description
---

Add mode to support delimited identifiers enclosed within double quotation


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e3ddbf197b 
  itests/src/test/resources/testconfiguration.properties c55f8db61a 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java 
1c0c62fb13 
  parser/pom.xml 18e0ad801d 
  parser/src/java/org/apache/hadoop/hive/ql/parse/ANTLRNoCaseStringStream.java 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/GenericHiveLexer.java 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 23f74ba05e 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexerParent.g 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexerStandard.g 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g b03b0989b8 
  parser/src/java/org/apache/hadoop/hive/ql/parse/Quotation.java PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AlterViewAddPartitionAnalyzer.java
 c1d2887ec8 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java 7820013ab0 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java 08aeeb2acd 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java 26c7a606bf 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
9bcc472a62 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
35f7ec674a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java 48c0a4a8ad 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java 6eb1ca2645 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java 
81540ba8c1 
  ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveUtils.java 
PRE-CREATION 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFCastFormat.java 
9afd5af2be 
  ql/src/test/queries/clientnegative/database_create_invalid_name.q 5d6749542b 
  ql/src/test/queries/clientpositive/quotedid_basic_standard.q PRE-CREATION 
  ql/src/test/queries/clientpositive/special_character_in_tabnames_1.q 
08df0d803c 
  ql/src/test/queries/clientpositive/special_character_in_tabnames_quotes_1.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/special_character_in_tabnames_quotes_2.q 
PRE-CREATION 
  ql/src/test/results/clientnegative/database_create_invalid_name.q.out 
4b2cd1e41b 
  
ql/src/test/results/clientpositive/llap/special_character_in_tabnames_quotes_1.q.out
 PRE-CREATION 
  
ql/src/test/results/clientpositive/llap/special_character_in_tabnames_quotes_2.q.out
 PRE-CREATION 
  ql/src/test/results/clientpositive/quotedid_basic_standard.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/special_character_in_tabnames_2.q.out 
b1a808a805 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 62f5773f9b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 77d34047a4 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 3f04abe47a 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java
 9d7cfd2a32 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
 6d82d794ca 


Diff: https://reviews.apache.org/r/72403/diff/2/

Changes: https://reviews.apache.org/r/72403/diff/1-2/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestCliDriver 
-Dqfile=quotedid_basic_standard.q,quote2.q,crtseltbl_serdeprops.q -pl 
itests/qtest -Pitests

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver 
-Dqfile=special_character_in_tabnames_quotes_1.q,special_character_in_tabnames_quotes_2.q,special_character_in_tabnames_1.q,special_character_in_tabnames_2.q
 -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa



[jira] [Created] (HIVE-23267) Reduce dependency on groovy

2020-04-21 Thread Ashutosh Chauhan (Jira)
Ashutosh Chauhan created HIVE-23267:
---

 Summary: Reduce dependency on groovy
 Key: HIVE-23267
 URL: https://issues.apache.org/jira/browse/HIVE-23267
 Project: Hive
  Issue Type: Improvement
  Components: Query Planning
Reporter: Ashutosh Chauhan
Assignee: Ashutosh Chauhan


Transitively pulled where its unneeded.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (HIVE-23266) Remove QueryWrapper from ObjectStore

2020-04-21 Thread David Mollitor (Jira)
David Mollitor created HIVE-23266:
-

 Summary: Remove QueryWrapper from ObjectStore
 Key: HIVE-23266
 URL: https://issues.apache.org/jira/browse/HIVE-23266
 Project: Hive
  Issue Type: Improvement
Reporter: David Mollitor
Assignee: David Mollitor


There is currently a utility called {{QueryWrapper}} that makes a normal 
{{Query}} auto-closable.  However, {{Query}} is now in fact already 
auto-closing, so there is no need for this class.  In trying to remove it, I 
realized that this wrapper was being passed around in pretty convoluted ways 
and also it was sometimes being created in a {{try-with-resources}} block but 
then never actually used in anyway.

Remove the {{QueryWrapper}} from the class and simplify some of the DB 
interactions.

https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L178



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (HIVE-23265) Duplicate rowsets are returned with Limit and Offset ste

2020-04-21 Thread Chiran Ravani (Jira)
Chiran Ravani created HIVE-23265:


 Summary: Duplicate rowsets are returned with Limit and Offset ste
 Key: HIVE-23265
 URL: https://issues.apache.org/jira/browse/HIVE-23265
 Project: Hive
  Issue Type: Bug
  Components: HiveServer2, Vectorization
Affects Versions: 3.1.2, 3.1.0
Reporter: Chiran Ravani
 Attachments: 00_0

We have a query which produces duplicate results even when there is no 
duplicate records in underlying tables.

Sample Query
{code:java}
select * from orderdatatest_ext order by col1 limit 1000,50
{code}

The problem appears when order by clause is used with col1 having non-unique 
rows. Apparently the duplicates are being produced during reducer phase of the 
query.
set hive.vectorized.execution.reduce.enabled=false does not cause the problem.

Data in table is as follows.
{code:java}
1,1
1,2
1,3
.
.
1,1500
{code}

Results with hive.vectorized.execution.reduce.enabled=true

{code:java}
+-+-+
| orderdatatest_ext.col1  | orderdatatest_ext.col2  |
+-+-+
| 1   | 1001|
| 1   | 1002|
| 1   | 1003|
| 1   | 1004|
| 1   | 1005|
| 1   | 1006|
| 1   | 1007|
| 1   | 1008|
| 1   | 1009|
| 1   | 1010|
| 1   | 1011|
| 1   | 1012|
| 1   | 1013|
| 1   | 1014|
| 1   | 1015|
| 1   | 1016|
| 1   | 1017|
| 1   | 1018|
| 1   | 1019|
| 1   | 1020|
| 1   | 1021|
| 1   | 1022|
| 1   | 1023|
| 1   | 1024|
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
| 1   | 1   |
+-+-+
{code}

Results with hive.vectorized.execution.reduce.enabled=false

{code:java}
+-+-+
| orderdatatest_ext.col1  | orderdatatest_ext.col2  |
+-+-+
| 1   | 1001|
| 1   | 1002|
| 1   | 1003|
| 1   | 1004|
| 1   | 1005|
| 1   | 1006|
| 1   | 1007|
| 1   | 1008|
| 1   | 1009|
| 1   | 1010|
| 1   | 1011|
| 1   | 1012|
| 1   | 1013|
| 1   | 1014|
| 1   | 

Review Request 72403: HIVE-19064

2020-04-21 Thread Krisztian Kasa

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72403/
---

Review request for hive and Jesús Camacho Rodríguez.


Bugs: HIVE-19064
https://issues.apache.org/jira/browse/HIVE-19064


Repository: hive-git


Description
---

Add mode to support delimited identifiers enclosed within double quotation


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e3ddbf197b 
  itests/src/test/resources/testconfiguration.properties c55f8db61a 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java 
1c0c62fb13 
  parser/pom.xml 18e0ad801d 
  parser/src/java/org/apache/hadoop/hive/ql/parse/ANTLRNoCaseStringStream.java 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/GenericHiveLexer.java 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/GenericHiveParser.java 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 23f74ba05e 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexerParent.g 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexerStandard.g 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g b03b0989b8 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParserParent.g 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParserStandard.g 
PRE-CREATION 
  parser/src/java/org/apache/hadoop/hive/ql/parse/Quotation.java PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/add/AlterViewAddPartitionAnalyzer.java
 c1d2887ec8 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java 7820013ab0 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveLockObject.java 08aeeb2acd 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java 26c7a606bf 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
9bcc472a62 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
35f7ec674a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java 48c0a4a8ad 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java 6eb1ca2645 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCastFormat.java 
81540ba8c1 
  ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveUtils.java 
PRE-CREATION 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFCastFormat.java 
9afd5af2be 
  ql/src/test/queries/clientnegative/database_create_invalid_name.q 5d6749542b 
  ql/src/test/queries/clientpositive/quotedid_basic_standard.q PRE-CREATION 
  ql/src/test/queries/clientpositive/special_character_in_tabnames_1.q 
08df0d803c 
  ql/src/test/queries/clientpositive/special_character_in_tabnames_quotes_1.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/special_character_in_tabnames_quotes_2.q 
PRE-CREATION 
  ql/src/test/results/clientnegative/database_create_invalid_name.q.out 
4b2cd1e41b 
  
ql/src/test/results/clientpositive/llap/special_character_in_tabnames_quotes_1.q.out
 PRE-CREATION 
  
ql/src/test/results/clientpositive/llap/special_character_in_tabnames_quotes_2.q.out
 PRE-CREATION 
  ql/src/test/results/clientpositive/quotedid_basic_standard.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/special_character_in_tabnames_2.q.out 
b1a808a805 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 62f5773f9b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 77d34047a4 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 3f04abe47a 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java
 9d7cfd2a32 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
 6d82d794ca 


Diff: https://reviews.apache.org/r/72403/diff/1/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestCliDriver 
-Dqfile=quotedid_basic_standard.q,quote2.q,crtseltbl_serdeprops.q -pl 
itests/qtest -Pitests

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver 
-Dqfile=special_character_in_tabnames_quotes_1.q,special_character_in_tabnames_quotes_2.q,special_character_in_tabnames_1.q,special_character_in_tabnames_2.q
 -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa



Re: Review Request 72392: HIVE-23103 Oracle statement batching

2020-04-21 Thread Marton Bod

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72392/#review220401
---


Ship it!




Ship It!

- Marton Bod


On April 21, 2020, 12:41 p.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72392/
> ---
> 
> (Updated April 21, 2020, 12:41 p.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Marton Bod.
> 
> 
> Bugs: HIVE-23103
> https://issues.apache.org/jira/browse/HIVE-23103
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Examine how to really get better performance for oracle statement batches.
> 
> Oracle JDBC doc describes:
> 
> The Oracle implementation of standard update batching does not implement true 
> batching for generic statements and callable statements. Even though Oracle 
> JDBC supports the use of standard batching for Statement and 
> CallableStatement objects, you are unlikely to see performance improvement.
> 
> I would look for connection properties to set, so it is handled anyway, or if 
> not, then use:
> 
> begin
>   query1;
>   query2;
>   query3;
> end;
> to we will have only a single roundtrip for the db.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d080df417b 
> 
> 
> Diff: https://reviews.apache.org/r/72392/diff/2/
> 
> 
> Testing
> ---
> 
> Baseline:
> Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
> Error  Units
> TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  42.988 ± 
> 4.569  ms/op
> TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  45.029 ± 
> 4.686  ms/op
> 
> After patch:
> Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
> Error  Units
> TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  36.208 ± 
> 3.869  ms/op
> TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  37.038 ± 
> 3.746  ms/op
> 
> 
> Thanks,
> 
> Peter Vary
> 
>



[jira] [Created] (HIVE-23264) Make partition_wise_fileformat12.q deterministic with order by clauses

2020-04-21 Thread Miklos Gergely (Jira)
Miklos Gergely created HIVE-23264:
-

 Summary: Make partition_wise_fileformat12.q deterministic with 
order by clauses
 Key: HIVE-23264
 URL: https://issues.apache.org/jira/browse/HIVE-23264
 Project: Hive
  Issue Type: Bug
  Components: Hive
Reporter: Miklos Gergely
Assignee: Miklos Gergely






--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.
> 
> Denys Kuzmenko wrote:
> ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?
> 
> Peter Varga wrote:
> No, if there is no record, the max will return one row with a null value
> 
> Denys Kuzmenko wrote:
> ok, got it, in this case why are we doing throw new 
> IllegalStateException("Scalar query returned no rows?!?!!")?

removed that.


- Peter


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/#review220367
---


On April 21, 2020, 12:51 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 12:51 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  e7910c1c5d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> 

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 699 (patched)
> > 
> >
> > openTxns(dbConn, stmt, rqst) is used in replTableWriteIdState as well. 
> > Do we have to check there for timeout as well?
> 
> Peter Varga wrote:
> This is an alarming catch. What kind of queries run on a replication. I 
> put there the table lock, but it will be a rather long synchronized block. On 
> the other hand aren't the replication tasks somehow serialized? If there are 
> no select queries on replication the timeout is not needed, if there are, we 
> might have a problem, because we do a lot of think before the commit, where 
> we can measure.
> And the most seriuos problem, if I see correctly, the replication will 
> use this newly opened transactionId-s to write into the TXN_TO_WRITE_ID, 
> wouldn't be a problem if this will be different than the original value?

As we discussed, the txn_id change is not a problem, and we don't need either 
the timeout check or the txnLock, because all of the opened transactions will 
be aborted in the same rdbms transaction


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 780 (patched)
> > 
> >
> > TXN_META_INFO? What is it used for before? Or is it new? Could we use a 
> > specific "state" for example?
> 
> Denys Kuzmenko wrote:
> My understanding was, it's kinda marker to flag the new txn entries and 
> get their ids.
> 
> Peter Varga wrote:
> It is a marker, I just used an existing field. It is updated to null 
> later.

change it to txn_state


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 641 (original), 783 (patched)
> > 
> >
> > Could we use a correct PreparedStatement, with the values set without 
> > the "hacky" quoting?
> > 
> > Like: pstmt.setXX for state/user/host/type/metainfo?
> > 
> > Or does this have a noticable performance gain?
> 
> Peter Varga wrote:
> Changed the metainfo select back and update to preparedstatements. The 
> original insert will entirely change in your batch insert patch if I know 
> correctly, so I will leave that.

incorporated your batchinsert patch also


- Peter


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/#review220369
---


On April 21, 2020, 12:51 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 12:51 p.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> 

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/
---

(Updated April 21, 2020, 12:51 p.m.)


Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
1d211857bf 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 bb29410e7d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 e7910c1c5d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/72388/diff/3/

Changes: https://reviews.apache.org/r/72388/diff/2-3/


Testing
---


Thanks,

Peter Varga



Re: Review Request 72392: HIVE-23103 Oracle statement batching

2020-04-21 Thread Peter Vary via Review Board


> On ápr. 21, 2020, 7:51 de, Marton Bod wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
> > Lines 662 (patched)
> > 
> >
> > Once this part executes, we wouldn't have 'begin' for the next batch, 
> > no? Also, the sb would need to be cleared I think

good find!
Fixed it


- Peter


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72392/#review220387
---


On ápr. 21, 2020, 12:41 du, Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72392/
> ---
> 
> (Updated ápr. 21, 2020, 12:41 du)
> 
> 
> Review request for hive, Denys Kuzmenko and Marton Bod.
> 
> 
> Bugs: HIVE-23103
> https://issues.apache.org/jira/browse/HIVE-23103
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Examine how to really get better performance for oracle statement batches.
> 
> Oracle JDBC doc describes:
> 
> The Oracle implementation of standard update batching does not implement true 
> batching for generic statements and callable statements. Even though Oracle 
> JDBC supports the use of standard batching for Statement and 
> CallableStatement objects, you are unlikely to see performance improvement.
> 
> I would look for connection properties to set, so it is handled anyway, or if 
> not, then use:
> 
> begin
>   query1;
>   query2;
>   query3;
> end;
> to we will have only a single roundtrip for the db.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d080df417b 
> 
> 
> Diff: https://reviews.apache.org/r/72392/diff/2/
> 
> 
> Testing
> ---
> 
> Baseline:
> Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
> Error  Units
> TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  42.988 ± 
> 4.569  ms/op
> TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  45.029 ± 
> 4.686  ms/op
> 
> After patch:
> Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
> Error  Units
> TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  36.208 ± 
> 3.869  ms/op
> TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  37.038 ± 
> 3.746  ms/op
> 
> 
> Thanks,
> 
> Peter Vary
> 
>



Re: Review Request 72392: HIVE-23103 Oracle statement batching

2020-04-21 Thread Peter Vary via Review Board

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72392/
---

(Updated ápr. 21, 2020, 12:41 du)


Review request for hive, Denys Kuzmenko and Marton Bod.


Changes
---

Addessed Marton's comment


Bugs: HIVE-23103
https://issues.apache.org/jira/browse/HIVE-23103


Repository: hive-git


Description
---

Examine how to really get better performance for oracle statement batches.

Oracle JDBC doc describes:

The Oracle implementation of standard update batching does not implement true 
batching for generic statements and callable statements. Even though Oracle 
JDBC supports the use of standard batching for Statement and CallableStatement 
objects, you are unlikely to see performance improvement.

I would look for connection properties to set, so it is handled anyway, or if 
not, then use:

begin
  query1;
  query2;
  query3;
end;
to we will have only a single roundtrip for the db.


Diffs (updated)
-

  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 bb29410e7d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 d080df417b 


Diff: https://reviews.apache.org/r/72392/diff/2/

Changes: https://reviews.apache.org/r/72392/diff/1-2/


Testing
---

Baseline:
Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
Error  Units
TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  42.988 ± 
4.569  ms/op
TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  45.029 ± 
4.686  ms/op

After patch:
Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
Error  Units
TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  36.208 ± 
3.869  ms/op
TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  37.038 ± 
3.746  ms/op


Thanks,

Peter Vary



Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Denys Kuzmenko via Review Board


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.
> 
> Denys Kuzmenko wrote:
> ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?
> 
> Peter Varga wrote:
> No, if there is no record, the max will return one row with a null value

ok, got it, in this case why are we doing throw new 
IllegalStateException("Scalar query returned no rows?!?!!")?


- Denys


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/#review220367
---


On April 21, 2020, 7:53 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 7:53 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  e7910c1c5d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
> 

Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-21 Thread Peter Varga via Review Board


> On April 17, 2020, 7:03 p.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
> > Lines 67 (patched)
> > 
> >
> > Why is this needed?

These remained here because of some failing tests, but I was able to fix all of 
those, so I removed it.


- Peter


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72380/#review220352
---


On April 21, 2020, 10:22 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72380/
> ---
> 
> (Updated April 21, 2020, 10:22 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Zoltan Chovan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> In the final version the prepDb creates the transactional tables with the 
> init schema (and ignores the others if they exists). The cleanDb resets the 
> database to the starting point. So between the test cases the cleanDb call is 
> enough. If the prepDb is called unneccessary it will just check if the txns 
> table exist and then return, so it will be fast
> 
> 
> Diffs
> -
> 
>   contrib/src/test/queries/clientpositive/url_hook.q 512e579fb8 
>   itests/hive-blobstore/pom.xml 09955c55f3 
>   itests/qtest-accumulo/pom.xml a35d2a8a10 
>   itests/qtest-druid/pom.xml cc0cceff68 
>   itests/qtest-kudu/pom.xml f23399fa37 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java 
> fcfc79059a 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
>  5b08f8b894 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java
>  b86d736a89 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 
> 38e4bac2cc 
>   pom.xml b29c06c69e 
>   ql/pom.xml a0e77a1d32 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
>  ebe4880e3a 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
>  3e56ad513c 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  e7910c1c5d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
>  5f3db52c2f 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
>  c537d95470 
> 
> 
> Diff: https://reviews.apache.org/r/72380/diff/2/
> 
> 
> Testing
> ---
> 
> On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
> minutes.
> 
> 
> Thanks,
> 
> Peter Varga
> 
>



Re: Review Request 72387: Locks: Add new lock implementations for always zero-wait readers

2020-04-21 Thread Denys Kuzmenko via Review Board

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72387/
---

(Updated April 21, 2020, 12:14 p.m.)


Review request for hive, Marton Bod and Peter Vary.


Changes
---

added quotes


Bugs: HIVE-19369
https://issues.apache.org/jira/browse/HIVE-19369


Repository: hive-git


Description
---

Hive Locking with Micro-managed and full-ACID tables needs a better locking 
implementation which allows for no-wait readers always.

EXCL_DROP
EXCL_WRITE
SHARED_WRITE
SHARED_READ

Short write-up

EXCL_DROP is a "drop partition" or "drop table" and waits for all others to exit
EXCL_WRITE excludes all writes and will wait for all existing SHARED_WRITE to 
exit.
SHARED_WRITE allows all SHARED_WRITES to go through, but will wait for an 
EXCL_WRITE & EXCL_DROP (waiting so that you can do drop + insert in different 
threads).

SHARED_READ does not wait for any lock - it fails fast for a pending EXCL_DROP, 
because even if there is an EXCL_WRITE or SHARED_WRITE pending, there's no 
semantic reason to wait for them to succeed before going ahead with a 
SHARED_WRITE.

a select * => SHARED_READ
an insert into => SHARED_WRITE
an insert overwrite or MERGE => EXCL_WRITE
a drop table => EXCL_DROP


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 16bae920df 
  ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 77878ca40b 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
1d211857bf 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  ql/src/test/queries/clientpositive/explain_locks.q 3c11560c5f 
  ql/src/test/results/clientpositive/llap/explain_locks.q.out 3183533478 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/LockRequestBuilder.java
 22902a9c20 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/LockTypeComparator.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 d080df417b 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/LockTypeUtil.java
 f928bf781b 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/LockTypeUtilTest.java
 3d69a6e7dc 


Diff: https://reviews.apache.org/r/72387/diff/4/

Changes: https://reviews.apache.org/r/72387/diff/3-4/


Testing
---

Added number of tests under TestDbTxnManager2, TestTxnHandler & extended 
explain_locks.q test.


Thanks,

Denys Kuzmenko



Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.
> 
> Denys Kuzmenko wrote:
> ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?

No, if there is no record, the max will return one row with a null value


- Peter


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/#review220367
---


On April 21, 2020, 7:53 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 7:53 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  e7910c1c5d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
>  a6d22d19ef 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
>  0b070e19ac 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
>  PRE-CREATION 
> 
> 
> Diff: 

Re: Review Request 72379: "get_splits" udf ignores limit constraint when creating splits

2020-04-21 Thread Adesh Rao


> On April 20, 2020, 2:19 a.m., Sankar Hariappan wrote:
> > itests/hive-unit/src/test/java/org/apache/hive/jdbc/AbstractTestJdbcGenericUDTFGetSplits.java
> > Lines 181 (patched)
> > 
> >
> > Add test for limit with predicates, groupby queries as well. Also, make 
> > sure, we have partitioned table too.
> 
> Adesh Rao wrote:
> Added queries with predicates and groupby. Since the existing test suite 
> does not have partitioned table, need to go through other tests to figure out 
> the standard way to generate a partitioned table.

done.


- Adesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72379/#review220363
---


On April 21, 2020, 11:17 a.m., Adesh Rao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72379/
> ---
> 
> (Updated April 21, 2020, 11:17 a.m.)
> 
> 
> Review request for hive and Sankar Hariappan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Since limit constraint was ignored while creating splits. If multiple llap 
> daemons execute splits, output contains more rows than specified by limit 
> constraint.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/AbstractTestJdbcGenericUDTFGetSplits.java
>  8cbca69737 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcGenericUDTFGetSplits.java
>  defbe78802 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcGenericUDTFGetSplits2.java
>  330174513c 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 
> 00a6c89b1e 
> 
> 
> Diff: https://reviews.apache.org/r/72379/diff/3/
> 
> 
> Testing
> ---
> 
> Waiting for Hive QA run.
> 
> 
> Thanks,
> 
> Adesh Rao
> 
>



[jira] [Created] (HIVE-23263) Add fix order to cbo_rp_limit.q queries + improve readability

2020-04-21 Thread Miklos Gergely (Jira)
Miklos Gergely created HIVE-23263:
-

 Summary: Add fix order to cbo_rp_limit.q queries + improve 
readability
 Key: HIVE-23263
 URL: https://issues.apache.org/jira/browse/HIVE-23263
 Project: Hive
  Issue Type: Bug
  Components: Hive
Reporter: Miklos Gergely
Assignee: Miklos Gergely
 Fix For: 4.0.0






--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Re: Review Request 72379: "get_splits" udf ignores limit constraint when creating splits

2020-04-21 Thread Adesh Rao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72379/
---

(Updated April 21, 2020, 11:17 a.m.)


Review request for hive and Sankar Hariappan.


Changes
---

Added test for partitioned table


Repository: hive-git


Description
---

Since limit constraint was ignored while creating splits. If multiple llap 
daemons execute splits, output contains more rows than specified by limit 
constraint.


Diffs (updated)
-

  
itests/hive-unit/src/test/java/org/apache/hive/jdbc/AbstractTestJdbcGenericUDTFGetSplits.java
 8cbca69737 
  
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcGenericUDTFGetSplits.java
 defbe78802 
  
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcGenericUDTFGetSplits2.java
 330174513c 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 
00a6c89b1e 


Diff: https://reviews.apache.org/r/72379/diff/3/

Changes: https://reviews.apache.org/r/72379/diff/2-3/


Testing
---

Waiting for Hive QA run.


Thanks,

Adesh Rao



Re: Review Request 72378: HIVE-23201: Improve logging in locking

2020-04-21 Thread Denys Kuzmenko via Review Board


> On April 21, 2020, 10:38 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 4427 (patched)
> > 
> >
> > I would keep as it was before. no need to modify the query

Maybe, just refrase "Failed to acquire lock: (extLockId={}, intLockId={}) as 
it's aleady taken by: (extLockId={}, intLockId={})"


- Denys


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72378/#review220389
---


On April 17, 2020, 11:30 a.m., Marton Bod wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72378/
> ---
> 
> (Updated April 17, 2020, 11:30 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-23201: Improve logging in locking
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java 4b6bc3e1e3 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d080df417b 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnUtils.java
>  b3a1f826bb 
> 
> 
> Diff: https://reviews.apache.org/r/72378/diff/2/
> 
> 
> Testing
> ---
> 
> Green build: https://builds.apache.org/job/PreCommit-HIVE-Build/21717/
> 
> 
> Thanks,
> 
> Marton Bod
> 
>



Re: Review Request 72378: HIVE-23201: Improve logging in locking

2020-04-21 Thread Denys Kuzmenko via Review Board

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72378/#review220389
---



LGTM, just number of comments


ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbLockManager.java
Lines 143 (patched)


Maybe use log.error here?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2413 (original), 2423 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2438 (original), 2449 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2633 (original), 2644 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2679 (original), 2692 (patched)


what's the benefit of this?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2693 (original), 2707 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2747 (original), 2761 (patched)


What's the benefit of this?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 2773 (original), 2787 (patched)


log.error, start with capitalized



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 4427 (patched)


I would keep as it was before. no need to modify the query



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4458 (original), 4476 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4481 (original), 4492 (patched)


What's the purpose of this?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4502 (original), 4513 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4527 (original), 4536 (patched)


log.error



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4717 (original), 4726 (patched)


Why not Prepared statement?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4728 (original), 4731 (patched)


could we use Optional here?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4742 (original), 4742 (patched)


Why not Prepared statement?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4771 (original), 4764 (patched)


Why not Prepared statement?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4809 (original), 4799 (patched)


I would refrase like
Deleted {} locks due to timed-out. Lock ids: {}



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Line 4817 (original), 4804 (patched)


Do we need "due to" if we supply exception.


- Denys Kuzmenko


On April 17, 2020, 11:30 a.m., Marton Bod wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72378/
> ---
> 
> (Updated April 17, 2020, 11:30 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 

Re: Review Request 72380: HIVE-23207 Create integration tests for TxnManager for different rdbms metastores

2020-04-21 Thread Peter Varga via Review Board

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72380/
---

(Updated April 21, 2020, 10:22 a.m.)


Review request for hive, Denys Kuzmenko and Zoltan Chovan.


Repository: hive-git


Description
---

In the final version the prepDb creates the transactional tables with the init 
schema (and ignores the others if they exists). The cleanDb resets the database 
to the starting point. So between the test cases the cleanDb call is enough. If 
the prepDb is called unneccessary it will just check if the txns table exist 
and then return, so it will be fast


Diffs (updated)
-

  contrib/src/test/queries/clientpositive/url_hook.q 512e579fb8 
  itests/hive-blobstore/pom.xml 09955c55f3 
  itests/qtest-accumulo/pom.xml a35d2a8a10 
  itests/qtest-druid/pom.xml cc0cceff68 
  itests/qtest-kudu/pom.xml f23399fa37 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java 
fcfc79059a 
  
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
 5b08f8b894 
  
itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestMetaStoreHandler.java 
b86d736a89 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 38e4bac2cc 
  pom.xml b29c06c69e 
  ql/pom.xml a0e77a1d32 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
1d211857bf 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandlerNoConnectionPool.java
 ebe4880e3a 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/ITestDbTxnManager.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DatabaseProduct.java
 3e56ad513c 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 bb29410e7d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 e7910c1c5d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java
 5f3db52c2f 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mysql.java
 c537d95470 


Diff: https://reviews.apache.org/r/72380/diff/2/

Changes: https://reviews.apache.org/r/72380/diff/1-2/


Testing
---

On my machine the 50 tests in TestDbTxnManager2 on postgres runs under 5 
minutes.


Thanks,

Peter Varga



Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Denys Kuzmenko via Review Board


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 172 (original), 225 (patched)
> > 
> >
> > Please rebase, this code is no longer here
> 
> Peter Varga wrote:
> which commit removed this? I don't see it.

oh, sorry, I confused it with the lock type. Not an issue ignore.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?
> 
> Peter Varga wrote:
> It should not be null, but the constraint is that there must be always 
> one record in the table, not a nonnull column.

ok, don't you cover this with if (!maxTxnIdRs.next()) { .. } ?


- Denys


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/#review220367
---


On April 21, 2020, 7:53 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 21, 2020, 7:53 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   
> ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  a874121e12 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  e7910c1c5d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>  2e0177723d 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
>  9f3951575b 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
>  0512a45cad 
>   
> standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
>  4b82e36ab4 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
>  db398e5f66 
>   
> standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
>  1be83fc4a9 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
>  e6e30160af 
>   
> standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
>  b90cecb173 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
>  c1a1629548 
>   
> 

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/
---

(Updated April 21, 2020, 7:53 a.m.)


Review request for hive and Peter Vary.


Repository: hive-git


Description
---

* Use sequences for TXN_ID generation.
  * Make it possible to open transactions in parallel
  * drop Oracle 11g support


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 37a5862791 
  
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java 
15fcfc0e35 
  ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
1d211857bf 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
6525ffc00a 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 1435269ed3 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/DbTxnManagerEndToEndTestBase.java 
PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
73d3b91585 
  
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerIsolationProperties.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
1151466f8c 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 a874121e12 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
 49b737ecf9 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
 2344c2d5f6 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
 bb29410e7d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
 e7910c1c5d 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
 87130a519d 
  
standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
 1ace9d3ef0 
  
standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
 8a3cd56658 
  
standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
 2e0177723d 
  
standalone-metastore/metastore-server/src/main/sql/mssql/upgrade-3.2.0-to-4.0.0.mssql.sql
 9f3951575b 
  
standalone-metastore/metastore-server/src/main/sql/mysql/hive-schema-4.0.0.mysql.sql
 0512a45cad 
  
standalone-metastore/metastore-server/src/main/sql/mysql/upgrade-3.2.0-to-4.0.0.mysql.sql
 4b82e36ab4 
  
standalone-metastore/metastore-server/src/main/sql/oracle/hive-schema-4.0.0.oracle.sql
 db398e5f66 
  
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-3.2.0-to-4.0.0.oracle.sql
 1be83fc4a9 
  
standalone-metastore/metastore-server/src/main/sql/postgres/hive-schema-4.0.0.postgres.sql
 e6e30160af 
  
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-3.2.0-to-4.0.0.postgres.sql
 b90cecb173 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/DbInstallBase.java
 c1a1629548 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/DatabaseRule.java
 a6d22d19ef 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Oracle.java
 0b070e19ac 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestOpenTxn.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/72388/diff/2/

Changes: https://reviews.apache.org/r/72388/diff/1-2/


Testing
---


Thanks,

Peter Varga



Re: Review Request 72392: HIVE-23103 Oracle statement batching

2020-04-21 Thread Marton Bod

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72392/#review220387
---




standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
Lines 662 (patched)


Once this part executes, we wouldn't have 'begin' for the next batch, no? 
Also, the sb would need to be cleared I think


- Marton Bod


On April 20, 2020, 2:53 p.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72392/
> ---
> 
> (Updated April 20, 2020, 2:53 p.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Marton Bod.
> 
> 
> Bugs: HIVE-23103
> https://issues.apache.org/jira/browse/HIVE-23103
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Examine how to really get better performance for oracle statement batches.
> 
> Oracle JDBC doc describes:
> 
> The Oracle implementation of standard update batching does not implement true 
> batching for generic statements and callable statements. Even though Oracle 
> JDBC supports the use of standard batching for Statement and 
> CallableStatement objects, you are unlikely to see performance improvement.
> 
> I would look for connection properties to set, so it is handled anyway, or if 
> not, then use:
> 
> begin
>   query1;
>   query2;
>   query3;
> end;
> to we will have only a single roundtrip for the db.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d080df417b 
> 
> 
> Diff: https://reviews.apache.org/r/72392/diff/1/
> 
> 
> Testing
> ---
> 
> Baseline:
> Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
> Error  Units
> TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  42.988 ± 
> 4.569  ms/op
> TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  45.029 ± 
> 4.686  ms/op
> 
> After patch:
> Benchmark(dbProduct)  (txnType)  Mode  Cnt   Score   
> Error  Units
> TxnHandlerBenchRunner.commitTxn   ORACLEDEFAULTss  100  36.208 ± 
> 3.869  ms/op
> TxnHandlerBenchRunner.commitTxn   ORACLE  READ_ONLYss  100  37.038 ± 
> 3.746  ms/op
> 
> 
> Thanks,
> 
> Peter Vary
> 
>



Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > This is a really big/scary change. I am really interested in the 
> > performance results! :)
> > Thanks for all the effort! Some questions below

Thank you for the review, i fixed most of them, but have a few questions.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Lines 563 (patched)
> > 
> >
> > Maybe insert the query instead of getting id and using again. Not very 
> > important, just asking...

I tried it at first, but it become really slow, when there are 5 txns in 
the table. Probably there is a way to do it properly, but this is running in a 
scheduled thread so it is not that essential I think.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 448 (original), 528 (patched)
> > 
> >
> > Just a question: This is so similar to getOpenTxnsInfo... Any way to 
> > use the same code with different query and different response handling?

I started it but I think we should do it in a next change, we have to create 
some inner datastructure, because both of responses now missing something from 
the other


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 699 (patched)
> > 
> >
> > openTxns(dbConn, stmt, rqst) is used in replTableWriteIdState as well. 
> > Do we have to check there for timeout as well?

This is an alarming catch. What kind of queries run on a replication. I put 
there the table lock, but it will be a rather long synchronized block. On the 
other hand aren't the replication tasks somehow serialized? If there are no 
select queries on replication the timeout is not needed, if there are, we might 
have a problem, because we do a lot of think before the commit, where we can 
measure.
And the most seriuos problem, if I see correctly, the replication will use this 
newly opened transactionId-s to write into the TXN_TO_WRITE_ID, wouldn't be a 
problem if this will be different than the original value?


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 780 (patched)
> > 
> >
> > TXN_META_INFO? What is it used for before? Or is it new? Could we use a 
> > specific "state" for example?
> 
> Denys Kuzmenko wrote:
> My understanding was, it's kinda marker to flag the new txn entries and 
> get their ids.

It is a marker, I just used an existing field. It is updated to null later.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 641 (original), 783 (patched)
> > 
> >
> > Could we use a correct PreparedStatement, with the values set without 
> > the "hacky" quoting?
> > 
> > Like: pstmt.setXX for state/user/host/type/metainfo?
> > 
> > Or does this have a noticable performance gain?

Changed the metainfo select back and update to preparedstatements. The original 
insert will entirely change in your batch insert patch if I know correctly, so 
I will leave that.


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 1168 (original), 1418 (patched)
> > 
> >
> > If we do not make such a fuss about the checking, just a simple assert 
> > instead, we might can inline the method here, since it is not used anywhere 
> > else? What do you think?

The commitTxns is already 200 lines long, if it's ok for you, i woudn't make it 
longer


> On April 20, 2020, 12:19 p.m., Peter Vary wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 2262 (patched)
> > 
> >
> > This is again a single query implemented with 3 SQL query and a java 
> > code. Am I right?

Yes, however it is a backgroung method. Does it worth to sacrifice the easier 
logging and debugging for some performance gain?


- Peter


---
This is an automatically generated e-mail. To 

Re: Review Request 72388: HIVE-23048 Use sequences for TXN_ID generation

2020-04-21 Thread Peter Varga via Review Board


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > LGTM, just a few comments

Thank you for the review, I fixed most of the issues, but have a few question.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
> > Line 552 (original), 560 (patched)
> > 
> >
> > Could we use this query as condition for delete? selete where exists 
> > (..)

I would rather not do that, this method is called from a scheduled backgroung 
thread, it don't have to be incredible fast, on the other hand, if we run it 
this way we log the txnid-s between the select and the delete. I would like to 
keep that.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Line 172 (original), 225 (patched)
> > 
> >
> > Please rebase, this code is no longer here

which commit removed this? I don't see it.


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 807 (patched)
> > 
> >
> > could we use stream instead of for loop: 
> > first = 
> > txnIds.stream().max(Comparator.comparing(Long::valueOf)).orElse(LONG_MAX)

actually turned out this is a dead code since the min history rebase, removed 
it alltogether


> On April 20, 2020, 10:40 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 902 (patched)
> > 
> >
> > Can it be null? Could we add constraint on db level instead of multiple 
> > checks in backend code?

It should not be null, but the constraint is that there must be always one 
record in the table, not a nonnull column.


- Peter


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72388/#review220367
---


On April 20, 2020, 7:25 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72388/
> ---
> 
> (Updated April 20, 2020, 7:25 a.m.)
> 
> 
> Review request for hive and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Use sequences for TXN_ID generation.
>   * Make it possible to open transactions in parallel
>   * drop Oracle 11g support
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> 37a5862791 
>   
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java
>  15fcfc0e35 
>   ql/src/test/org/apache/hadoop/hive/metastore/txn/TestTxnHandler.java 
> 1d211857bf 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java 2c13e8dd03 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands3.java 51b0fa336f 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java 
> 6525ffc00a 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 1435269ed3 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 73d3b91585 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManagerAcid.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java 
> 1151466f8c 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  f6097f7520 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/SQLGenerator.java
>  49b737ecf9 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  2344c2d5f6 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
>  bb29410e7d 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  d080df417b 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
>  87130a519d 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/hive-schema-4.0.0.derby.sql
>  1ace9d3ef0 
>   
> standalone-metastore/metastore-server/src/main/sql/derby/upgrade-3.2.0-to-4.0.0.derby.sql
>  8a3cd56658 
>   
> standalone-metastore/metastore-server/src/main/sql/mssql/hive-schema-4.0.0.mssql.sql
>