Re: Review Request 72465: HIVE-23340 TxnHandler cleanup

[Denys Kuzmenko via Review Board]

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


Ship it!




Ship It!

- Denys Kuzmenko


On May 20, 2020, 12:24 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72465/
> ---
> 
> (Updated May 20, 2020, 12:24 p.m.)
> 
> 
> Review request for hive and Denys Kuzmenko.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Merge getOpenTxns and getOpenTxnInfo to avoid code duplication
>   * Remove TxnStatus character constants and use the enum values
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  d2efc595a5 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OpenTxn.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OpenTxnList.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OperationType.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  89ddccbbda 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStatus.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72465/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72465: HIVE-23340 TxnHandler cleanup

[Denys Kuzmenko via Review Board]

> On May 18, 2020, 7:33 a.m., Denys Kuzmenko wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStatus.java
> > Lines 32 (patched)
> > 
> >
> > Could you please refactor here as well LockTypeUtil to LockType enum.
> 
> Denys Kuzmenko wrote:
> move to txn package + overwrite toString(), this would allow to get rid 
> of static exclWrite, sharedWrite, sharedRead methods
> 
> Peter Varga wrote:
> The LockType enum is part of the thrift interface. I could duplicate it, 
> but I am not sure it will be cleaner that way. I don't know why they went 
> with this weird encoding, that the enums are represented as ints in the 
> interface and as char-s in the db. So many unnecessary switch between the two 
> representation.

ok, let's address this in diff patch


- Denys


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


On May 20, 2020, 12:24 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72465/
> ---
> 
> (Updated May 20, 2020, 12:24 p.m.)
> 
> 
> Review request for hive and Denys Kuzmenko.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Merge getOpenTxns and getOpenTxnInfo to avoid code duplication
>   * Remove TxnStatus character constants and use the enum values
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  d2efc595a5 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OpenTxn.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OpenTxnList.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OperationType.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  89ddccbbda 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStatus.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72465/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72465: HIVE-23340 TxnHandler cleanup

[Denys Kuzmenko via Review Board]

- Denys


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


On May 20, 2020, 12:24 p.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72465/
> ---
> 
> (Updated May 20, 2020, 12:24 p.m.)
> 
> 
> Review request for hive and Denys Kuzmenko.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * Merge getOpenTxns and getOpenTxnInfo to avoid code duplication
>   * Remove TxnStatus character constants and use the enum values
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
>  d2efc595a5 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OpenTxn.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OpenTxnList.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/OperationType.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  89ddccbbda 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStatus.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72465/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72488: HIVE-23413: New config to skip all locks

[Denys Kuzmenko via Review Board]

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2737 (patched)


I would name config property - HIVE_TXN_DISABLE_LOCKS, to give more 
clarification on  its purpose.


- Denys Kuzmenko


On May 19, 2020, 7:38 a.m., Peter Varga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72488/
> ---
> 
> (Updated May 19, 2020, 7:38 a.m.)
> 
> 
> Review request for hive, Denys Kuzmenko and Peter Vary.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> From time-to-time some query is blocked on locks which should not.
> 
> To have a quick workaround for this we should have a config which the user 
> can set in the session to disable acquiring/checking locks, so we can provide 
> it immediately and then later investigate and fix the root cause.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 60ae06a49a 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java deaab89c1f 
>   ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java 
> 2adabe7058 
> 
> 
> Diff: https://reviews.apache.org/r/72488/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Peter Varga
> 
>

Re: Review Request 72433: Extract Create View analyzer from SemanticAnalyzer

[Miklos Gergely]

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

(Updated June 3, 2020, 4:35 a.m.)


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


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


Repository: hive-git


Description
---

Create View commands are not queries, but commands which have queries as a part 
of them. Therefore a separate CreateViewAnalyzer is needed which uses 
SemanticAnalyer to analyze it's query.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java d732004e51 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 768a3a17a7 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java 
d1f36945fb 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java 
f7952a5cc1 
  ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java 792e331884 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 377e8280e5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java da443f419c 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8238a2a4a2 
  ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a 
  ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272 
  ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 2fb452bea5 
  ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596 
  ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46 
  ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292 
  ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d 
  ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e 
  ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06 
  ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e 
  ql/src/test/results/clientnegative/masking_mv.q.out 926c5062ce 
  ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2 
  ql/src/test/results/clientpositive/llap/create_view.q.out 52b77c7505 
  ql/src/test/results/clientpositive/llap/create_view_translate.q.out 
b5d464e716 
  ql/src/test/results/clientpositive/llap/explain_ddl.q.out 3cada1bbae 
  ql/src/test/results/clientpositive/llap/explainuser_1.q.out a2f241c470 
  ql/src/test/results/clientpositive/llap/lineage3.q.out c87d7c0c92 
  ql/src/test/results/clientpositive/llap/masking_mv.q.out 196688a17d 
  ql/src/test/results/clientpositive/llap/materialized_view_cluster.q.out 
e34147d1da 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out
 6e6ee34361 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out
 23489244bd 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out
 d4bf2a6d8b 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_time_window.q.out
 dc0e861863 
  
ql/src/test/results/clientpositive/llap/materialized_view_distribute_sort.q.out 
cdcc356ae3 
  
ql/src/test/results/clientpositive/llap/materialized_view_partition_cluster.q.out
 4d6de9d8f7 
  ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out 
e463b3cba6 
  ql/src/test/results/clientpositive/llap/materialized_view_partitioned_3.q.out 
8bdbf13d8a 
  ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4 
  
ql/src/test/results/clientpositive/llap/sketches_materialized_view_safety.q.out 
ff079587fd 
  ql/src/test/results/clientpositive/llap/union_top_level.q.out f846cb2fc1 
  ql/src/test/results/clientpositive/llap/vector_windowing.q.out ba31832201 
  ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 33546dd74f 
  ql

Re: Review Request 72433: Extract Create View analyzer from SemanticAnalyzer

[Miklos Gergely]

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

(Updated June 3, 2020, 2:54 a.m.)


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


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


Repository: hive-git


Description
---

Create View commands are not queries, but commands which have queries as a part 
of them. Therefore a separate CreateViewAnalyzer is needed which uses 
SemanticAnalyer to analyze it's query.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java d732004e51 
  parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 768a3a17a7 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLUtils.java b82fc5e91d 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewAnalyzer.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AbstractCreateViewDesc.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsAnalyzer.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsDesc.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/AlterViewAsOperation.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewOperation.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java 
d1f36945fb 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java 
f7952a5cc1 
  ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java 792e331884 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 377e8280e5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java da443f419c 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 9d94b6e2dd 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8238a2a4a2 
  ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 2350646c36 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 2f3fc6c50a 
  ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java c75829c272 
  ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 07bcef8ee3 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 2fb452bea5 
  ql/src/test/results/clientnegative/create_or_replace_view4.q.out 767cc77596 
  ql/src/test/results/clientnegative/create_view_failure3.q.out 8b79272f46 
  ql/src/test/results/clientnegative/create_view_failure5.q.out b7b3984292 
  ql/src/test/results/clientnegative/create_view_failure6.q.out 6d9fb6461d 
  ql/src/test/results/clientnegative/create_view_failure7.q.out 337dbe889e 
  ql/src/test/results/clientnegative/create_view_failure8.q.out cccb7e4b06 
  ql/src/test/results/clientnegative/create_view_failure9.q.out eac8cb489e 
  ql/src/test/results/clientnegative/selectDistinctStarNeg_1.q.out 9496e528e2 
  ql/src/test/results/clientpositive/llap/create_view.q.out 52b77c7505 
  ql/src/test/results/clientpositive/llap/create_view_translate.q.out 
b5d464e716 
  ql/src/test/results/clientpositive/llap/explain_ddl.q.out 3cada1bbae 
  ql/src/test/results/clientpositive/llap/explainuser_1.q.out a2f241c470 
  ql/src/test/results/clientpositive/llap/lineage3.q.out c87d7c0c92 
  ql/src/test/results/clientpositive/llap/masking_mv.q.out 196688a17d 
  ql/src/test/results/clientpositive/llap/materialized_view_cluster.q.out 
e34147d1da 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out
 6e6ee34361 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out
 23489244bd 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out
 d4bf2a6d8b 
  
ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_time_window.q.out
 dc0e861863 
  
ql/src/test/results/clientpositive/llap/materialized_view_distribute_sort.q.out 
cdcc356ae3 
  
ql/src/test/results/clientpositive/llap/materialized_view_partition_cluster.q.out
 4d6de9d8f7 
  ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out 
e463b3cba6 
  ql/src/test/results/clientpositive/llap/materialized_view_partitioned_3.q.out 
8bdbf13d8a 
  ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 3fc0074ed4 
  ql/src/test/results/clientpositive/llap/union_top_level.q.out f846cb2fc1 
  ql/src/test/results/clientpositive/llap/vector_windowing.q.out ba31832201 
  ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 33546dd74f 
  ql/src/test/results/clientpositive/tez/explainuser_3.q.out f1c245b671 


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

Changes: https://reviews.apache.org/r/724

Re: Review Request 72433: Extract Create View analyzer from SemanticAnalyzer

[Miklos Gergely]

> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java
> > Line 442 (original)
> > 
> >
> > Why is this error code going away? Even if it is not used probably 
> > makes sense to keep it around and mark it as deprecated for backwards 
> > reference.

I agree, put it back, and added deprecation with explanation.


> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
> > Lines 49 (patched)
> > 
> >
> > It seems more appropiate to update the display name too at this point.

I was planning to do that in the next patch, when I'm going to clean up 
materialized view creation, but it can be done here too. I've modified the 
patch.


> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateMaterializedViewDesc.java
> > Lines 67 (patched)
> > 
> >
> > // only used for materialized views
> > 
> > These comments can go away.

I was planning to do that in the next patch, when I'm going to clean up 
materialized view creation, but it can be done here too. I've modified the 
patch.


> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
> > Line 1818 (original), 1818 (patched)
> > 
> >
> > Can we make this protected again? I did not see any usage that is not 
> > by a subclass?

Sure, it's protected again.


> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 566 (patched)
> > 
> >
> > What is the goal of this block? Why is this needed now and it was not 
> > needed before? This seems more than a refactoring.
> > 
> > Fwiw there is some logic below executed when we create a view in 
> > _handleCreateViewDDL_.

We needed this before as well, this is what replaces handleCreateViewDDL for 
view creation. Please see the explanation below in the next issue about the new 
paradigm. From now on the condition

cboCtx.type == PreCboCtx.Type.VIEW

will not be true for CREATE VIEW commands as the SA won't "know" that it is 
parsing a query for a view creation. I've tried to move all DDL related things 
into the DDL analyzer (CreateViewAnalyzer), right now these two commands found 
here had to stay here, but the rest of handleCreateViewDDL is now happening in 
the CreateViewAnalyzer.

Just to be clear, in the near future I'm planning to remove handleCreateViewDDL 
completely, once CREATE MATERIALIZED VIEW command will have their own analyzer 
too, and they will use SA only to analyze their query in a similar fashion.


> On May 22, 2020, 10:18 p.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Line 1848 (original), 1854 (patched)
> > 
> >
> > Why are we changing this? We should not have to change this logic since 
> > isView semantics should not change.

isView was removed. When there is a CREATE VIEW command then it is parsed into 
an AST tree, which containes some info about the view (lets call it V) and a 
query (lets call it Q).

So far the whole tree was sent to the SemanticAnalyzer, kind of like saying 
that a CREATE VIEW command is just a special query. Then the SA identified that 
it is actually a CREATE VIEW command, so it created a CreateViewDesc for it, 
analyzing the V part, and saved it in the actual QB instance - no idea why 
there, I believe it just had to be put somewhere. Then the Q part was analyzed 
like every other query, and the results of this analysis were added to the 
CreateViewDesc, which could be always found in the QB.

After this change we'll have a new paradigm: a CREATE VIEW command is not a 
special query, it is a DDL, which contains a query. We'll have a 
CreateViewAnalyzer, which analyzes the V part, and as the Q part is a query, it 
sends the Q part, and only the Q part to a new SA. Ideally the SA would be 100% 
agnostic of what it is analyzing, whether it is a "regular" query, or a query 
of a CREATE VIEW command, but so far there are parts of the code which needs to 
know this. After this change it can't be done by looking at the QB if it 
contains a CreateViewDesc, but we still need this information, so I've 
introduced a forViewCreation global variable for now. I hope that in the future 
we'll find a way to rermove it, and make the SA totally agnostic

Re: Open old PRs

[Stamatis Zampetakis]

Hello,

I am very happy working with the new system. Many thanks Zoltan!

I find the bot a good idea and I think its worth trying it out.
One thing to watch out is the case where contributors are willing to push
their work forward but there are no available reviewers to look to each
case.
I think people will reply to the bot once or twice but I don't think they
will do it much longer so we could take this into account for the
configuration of the bot.

Regarding merge squash option there might be a small caveat. I don't know
if it is possible to retain the information about the person who performed
the merge.
According to the discussion in [1] it seems that the committer in this case
will appear to be the GitHub account.
This might not be a big problem for Hive since the reviewer's name is part
of the commit message so the credit and responsibility is not lost.

Best,
Stamatis

[1] https://github.com/isaacs/github/issues/1303



On Tue, Jun 2, 2020 at 9:26 PM Zoltan Haindrich  wrote:

>
>
> On 6/2/20 9:15 PM, David Mollitor wrote:
> > I use a personal account for GitHub and it's not synced with my official
> > Apache account.  How do I go about registering my Apache account with
> > GitHub so I can merge through their interface?
>
> IIRC I've linked my account by using this interface:
> https://gitbox.apache.org/setup/
>
> >
> > In the meanwhile, can you assist with a merge here? :)
> >
>
> sure; I think you should also add dmolli...@apache.org as a secondary
> email to your github account
>
> About the open pr stuff: I still think our best approach of handling those
> things would be to close most of that 400 or so PRs...easiest would be to
> install the bot (at
> least temporarily)
> https://issues.apache.org/jira/browse/HIVE-23590
> what do you think?
>
> cheers,
> Zoltan
>
>
> > https://github.com/apache/hive/pull/1045
> >
> > Thanks!
> >
> > On Tue, Jun 2, 2020 at 10:21 AM Zoltan Haindrich  wrote:
> >
> >>
> >>
> >> On 6/2/20 3:10 PM, David Mollitor wrote:
> >>> I think we might want to take one manual pass across the board.  It
> will
> >>> most likely take more than 7 days to get through them all, so it may be
> >>> closing things that are legitimate.
> >>
> >> yeah...a manual pass would be good; I went thru around 10 or so before
> >> I've wrote the first mail in this thread...
> >> and I definetly don't want to go thru 400 - so I would preffer the bot
> :D
> >>
> >>>
> >>> One low hanging fruit (that applied to one of my PRs).  The JIRA it was
> >>> associated with was already closed.  Is there a way to target those?
> >>
> >> yes; there might be certainly a lot of those...(that's why I've estimate
> >> to 1/3 to be applicable)
> >> but filtering out even this is an awful lot of work (or it might involve
> >> writing a "bot")...
> >> if it's important enough the contributor could reopen / rebase the
> patch.
> >> We could try to communicate the non-hostaile intention in the message
> >> placed by the bot.
> >> The current message is the stale PRs would get is:
> >> "This pull request has been automatically marked as stale because it has
> >> not had recent activity. It will be closed if no further activity
> occurs."
> >>
> >>> Also, I have submitted my first PR to test out the new system.  It
> >>> has passed tests.  Ashutoshc has generously provided a +1.  What's the
> >>> next step to get it merged into the master?  Do I download the patch
> from
> >>> Github and apply manually using my Apache credentials?  Is the "merge"
> >>> feature setup in Github?  As I understand it, GitHub is only mirroring
> >> the
> >>> Apache git system.  Whatever the process we need an update in the
> >>> HowToContribute docs.
> >>
> >> That's an interesting question; the github repo is linked to the apache
> >> repo - so you may push/merge/whatever on the github interface; it will
> work.
> >> Github supports 3 modes to merge PRs:
> >> * We should definetly disable the "merge" option as that will just
> create
> >> a internation railways station from our history :)
> >> * rebase doesn't make it easier for reviewier to keep track new
> >> changes...because the PR owner have to continuosly force push the branch
> >> * squash merge work great - and I remembered that it changes the author
> to
> >> the user pushing the "squash" button; however right now it seems that it
> >> changes the author to
> >> the "user who opened the pr" which looks good-enough for me!
> >> (I've added the neccessary .asf.yaml changes to the existing PR)
> >>
> >> cheers,
> >> Zoltan
> >>
> >>> https://github.com/apache/hive/pull/1045
> >>>
> >>
> https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-ApplyingaPatch
> >>>
> >>>
> >>> Thanks!
> >>>
> >>> On Tue, Jun 2, 2020 at 4:58 AM Zoltan Haindrich  wrote:
> >>>
>  I think to use "probot" we would need to ask infra to configure the
>  "probot" github app.
>  It seems to me that the stale plugin from github actions provides
> almost
>  the same functio

[jira] [Created] (HIVE-23596) Encode guaranteed task information in containerId

[Mustafa Iman (Jira)]

Mustafa Iman created HIVE-23596:
---

 Summary: Encode guaranteed task information in containerId
 Key: HIVE-23596
 URL: https://issues.apache.org/jira/browse/HIVE-23596
 Project: Hive
  Issue Type: Improvement
Reporter: Mustafa Iman
Assignee: Mustafa Iman


We should avoid calling LlapTaskScheduler to get initial isguaranteed flag for 
all the tasks. It causes arbitrary delays in sending tasks out. Since 
communicator is a single thread, any blocking there delays all the tasks.

There are [https://jira.apache.org/jira/browse/TEZ-4192] and 
[https://jira.apache.org/jira/browse/HIVE-23589] for a proper solution to this. 
However, that requires a Tez release which seems far right now. We can replace 
the current hack with another hack that does not require locking.



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

[jira] [Created] (HIVE-23595) Do not query task guaranteed status when wlm off

[Mustafa Iman (Jira)]

Mustafa Iman created HIVE-23595:
---

 Summary: Do not query task guaranteed status when wlm off
 Key: HIVE-23595
 URL: https://issues.apache.org/jira/browse/HIVE-23595
 Project: Hive
  Issue Type: Improvement
Reporter: Mustafa Iman
Assignee: Mustafa Iman


LlapTaskCommunicator queries scheduler for every task guaranteed status. When 
workload management is off it is always false. There is no need for the 
synchronous check.



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

HIVE-21894 review?

[Justin Leet]

I've had a PR out for awhile for SSL with the KafkaStorageHandler that
isn't plaintext table properties, that's been waiting for both general
review and specific feedback for a few questions (detailed on the PR
itself).

Would someone be able to help get this pushed across the finish line?

https://issues.apache.org/jira/browse/HIVE-21894
https://github.com/apache/hive/pull/839

Re: Open old PRs

[Zoltan Haindrich]

On 6/2/20 9:15 PM, David Mollitor wrote:

I use a personal account for GitHub and it's not synced with my official
Apache account.  How do I go about registering my Apache account with
GitHub so I can merge through their interface?


IIRC I've linked my account by using this interface: 
https://gitbox.apache.org/setup/



In the meanwhile, can you assist with a merge here? :)



sure; I think you should also add dmolli...@apache.org as a secondary email to 
your github account

About the open pr stuff: I still think our best approach of handling those things would be to close most of that 400 or so PRs...easiest would be to install the bot (at 
least temporarily)

https://issues.apache.org/jira/browse/HIVE-23590
what do you think?

cheers,
Zoltan



https://github.com/apache/hive/pull/1045

Thanks!

On Tue, Jun 2, 2020 at 10:21 AM Zoltan Haindrich  wrote:




On 6/2/20 3:10 PM, David Mollitor wrote:

I think we might want to take one manual pass across the board.  It will
most likely take more than 7 days to get through them all, so it may be
closing things that are legitimate.


yeah...a manual pass would be good; I went thru around 10 or so before
I've wrote the first mail in this thread...
and I definetly don't want to go thru 400 - so I would preffer the bot :D



One low hanging fruit (that applied to one of my PRs).  The JIRA it was
associated with was already closed.  Is there a way to target those?


yes; there might be certainly a lot of those...(that's why I've estimate
to 1/3 to be applicable)
but filtering out even this is an awful lot of work (or it might involve
writing a "bot")...
if it's important enough the contributor could reopen / rebase the patch.
We could try to communicate the non-hostaile intention in the message
placed by the bot.
The current message is the stale PRs would get is:
"This pull request has been automatically marked as stale because it has
not had recent activity. It will be closed if no further activity occurs."


Also, I have submitted my first PR to test out the new system.  It
has passed tests.  Ashutoshc has generously provided a +1.  What's the
next step to get it merged into the master?  Do I download the patch from
Github and apply manually using my Apache credentials?  Is the "merge"
feature setup in Github?  As I understand it, GitHub is only mirroring

the

Apache git system.  Whatever the process we need an update in the
HowToContribute docs.


That's an interesting question; the github repo is linked to the apache
repo - so you may push/merge/whatever on the github interface; it will work.
Github supports 3 modes to merge PRs:
* We should definetly disable the "merge" option as that will just create
a internation railways station from our history :)
* rebase doesn't make it easier for reviewier to keep track new
changes...because the PR owner have to continuosly force push the branch
* squash merge work great - and I remembered that it changes the author to
the user pushing the "squash" button; however right now it seems that it
changes the author to
the "user who opened the pr" which looks good-enough for me!
(I've added the neccessary .asf.yaml changes to the existing PR)

cheers,
Zoltan


https://github.com/apache/hive/pull/1045


https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-ApplyingaPatch



Thanks!

On Tue, Jun 2, 2020 at 4:58 AM Zoltan Haindrich  wrote:


I think to use "probot" we would need to ask infra to configure the
"probot" github app.
It seems to me that the stale plugin from github actions provides almost
the same functionaluty - as they seem to be more or less identical in
features I would go with the
latter.

I've opened a pr to enable stale on the hive repo:
by default it will mark as stale afte 60 days; and close it if there is

no

activity for another 7 days
https://github.com/apache/hive/pull/1049

cheers,
Zoltan


On 6/2/20 5:00 AM, Ashutosh Chauhan wrote:

How about using stalebot : https://github.com/probot/stale ?

On Mon, Jun 1, 2020 at 12:20 PM Zoltán Haindrich  wrote:




Hey David,

On June 1, 2020 3:52:05 PM GMT+02:00, David Mollitor <

dam6...@gmail.com



wrote:

Any idea how long it will take to run precomit on all existing PRs?


I'm not entirely sure, but a rough estimate could be:
* not every pr is mergeable; there are many which was already
merged/outdated/etc...lets estimate that 1/3 is mergeable
* every pr runs for at least 1 hours

this would mean 430/3*1/24 days of test execution which is at least 6

days.


I see little to no value in running tests on archaic prs.
We could also configure an automatism to close prs after some time of
inactivity
https://github.com/actions/stale/blob/master/README.md
The good side of this is that it will get rid of ancient prs; however

it

might seem rude to a contributor in case he is waiting for feedback or
something
Even with that argument I think we should configure it at least for a

few

days to get rid of the dangling prs of almost 

Re: Open old PRs

[David Mollitor]

Hey Zoltan,

I use a personal account for GitHub and it's not synced with my official
Apache account.  How do I go about registering my Apache account with
GitHub so I can merge through their interface?

In the meanwhile, can you assist with a merge here? :)

https://github.com/apache/hive/pull/1045

Thanks!

On Tue, Jun 2, 2020 at 10:21 AM Zoltan Haindrich  wrote:

>
>
> On 6/2/20 3:10 PM, David Mollitor wrote:
> > I think we might want to take one manual pass across the board.  It will
> > most likely take more than 7 days to get through them all, so it may be
> > closing things that are legitimate.
>
> yeah...a manual pass would be good; I went thru around 10 or so before
> I've wrote the first mail in this thread...
> and I definetly don't want to go thru 400 - so I would preffer the bot :D
>
> >
> > One low hanging fruit (that applied to one of my PRs).  The JIRA it was
> > associated with was already closed.  Is there a way to target those?
>
> yes; there might be certainly a lot of those...(that's why I've estimate
> to 1/3 to be applicable)
> but filtering out even this is an awful lot of work (or it might involve
> writing a "bot")...
> if it's important enough the contributor could reopen / rebase the patch.
> We could try to communicate the non-hostaile intention in the message
> placed by the bot.
> The current message is the stale PRs would get is:
> "This pull request has been automatically marked as stale because it has
> not had recent activity. It will be closed if no further activity occurs."
>
> > Also, I have submitted my first PR to test out the new system.  It
> > has passed tests.  Ashutoshc has generously provided a +1.  What's the
> > next step to get it merged into the master?  Do I download the patch from
> > Github and apply manually using my Apache credentials?  Is the "merge"
> > feature setup in Github?  As I understand it, GitHub is only mirroring
> the
> > Apache git system.  Whatever the process we need an update in the
> > HowToContribute docs.
>
> That's an interesting question; the github repo is linked to the apache
> repo - so you may push/merge/whatever on the github interface; it will work.
> Github supports 3 modes to merge PRs:
> * We should definetly disable the "merge" option as that will just create
> a internation railways station from our history :)
> * rebase doesn't make it easier for reviewier to keep track new
> changes...because the PR owner have to continuosly force push the branch
> * squash merge work great - and I remembered that it changes the author to
> the user pushing the "squash" button; however right now it seems that it
> changes the author to
> the "user who opened the pr" which looks good-enough for me!
> (I've added the neccessary .asf.yaml changes to the existing PR)
>
> cheers,
> Zoltan
>
> > https://github.com/apache/hive/pull/1045
> >
> https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-ApplyingaPatch
> >
> >
> > Thanks!
> >
> > On Tue, Jun 2, 2020 at 4:58 AM Zoltan Haindrich  wrote:
> >
> >> I think to use "probot" we would need to ask infra to configure the
> >> "probot" github app.
> >> It seems to me that the stale plugin from github actions provides almost
> >> the same functionaluty - as they seem to be more or less identical in
> >> features I would go with the
> >> latter.
> >>
> >> I've opened a pr to enable stale on the hive repo:
> >> by default it will mark as stale afte 60 days; and close it if there is
> no
> >> activity for another 7 days
> >> https://github.com/apache/hive/pull/1049
> >>
> >> cheers,
> >> Zoltan
> >>
> >>
> >> On 6/2/20 5:00 AM, Ashutosh Chauhan wrote:
> >>> How about using stalebot : https://github.com/probot/stale ?
> >>>
> >>> On Mon, Jun 1, 2020 at 12:20 PM Zoltán Haindrich  wrote:
> >>>
> 
> 
>  Hey David,
> 
>  On June 1, 2020 3:52:05 PM GMT+02:00, David Mollitor <
> dam6...@gmail.com
> >>>
>  wrote:
> > Any idea how long it will take to run precomit on all existing PRs?
> >
>  I'm not entirely sure, but a rough estimate could be:
>  * not every pr is mergeable; there are many which was already
>  merged/outdated/etc...lets estimate that 1/3 is mergeable
>  * every pr runs for at least 1 hours
> 
>  this would mean 430/3*1/24 days of test execution which is at least 6
> >> days.
> 
>  I see little to no value in running tests on archaic prs.
>  We could also configure an automatism to close prs after some time of
>  inactivity
>  https://github.com/actions/stale/blob/master/README.md
>  The good side of this is that it will get rid of ancient prs; however
> it
>  might seem rude to a contributor in case he is waiting for feedback or
>  something
>  Even with that argument I think we should configure it at least for a
> >> few
>  days to get rid of the dangling prs of almost a decade ! (pr#2 is
> >> opened in
>  2011)...
>  What do you think?
> 
>  cheers

[jira] [Created] (HIVE-23594) Map more windowing aggregate functions to calcite

[Zoltan Haindrich (Jira)]

Zoltan Haindrich created HIVE-23594:
---

 Summary: Map more windowing aggregate functions to calcite
 Key: HIVE-23594
 URL: https://issues.apache.org/jira/browse/HIVE-23594
 Project: Hive
  Issue Type: Sub-task
Reporter: Zoltan Haindrich
Assignee: Zoltan Haindrich


we have a few methods which are not mapped like CUME_DIST ; there might be a 
few more



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

[jira] [Created] (HIVE-23593) Schemainit fails with NoSuchFieldError

[Zoltan Haindrich (Jira)]

Zoltan Haindrich created HIVE-23593:
---

 Summary: Schemainit fails with NoSuchFieldError 
 Key: HIVE-23593
 URL: https://issues.apache.org/jira/browse/HIVE-23593
 Project: Hive
  Issue Type: Bug
Reporter: Zoltan Haindrich
Assignee: Zoltan Haindrich


the issue comes from a calcite related class ; it's very interesting because ql 
already has a shaded calcite
{code}
Caused by: java.lang.NoSuchFieldError: operands
at 
org.apache.hadoop.hive.ql.optimizer.calcite.translator.ExprNodeConverter.visitCall(ExprNodeConverter.java:192)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.optimizer.calcite.translator.ExprNodeConverter.visitCall(ExprNodeConverter.java:98)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at org.apache.calcite.rex.RexCall.accept(RexCall.java:191) 
~[calcite-core-1.21.0.jar:1.21.0]
at 
org.apache.hadoop.hive.ql.optimizer.calcite.HiveRexExecutorImpl.reduce(HiveRexExecutorImpl.java:56)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.parse.type.HiveFunctionHelper.foldExpression(HiveFunctionHelper.java:544)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.parse.type.HiveFunctionHelper.createConstantObjectInspector(HiveFunctionHelper.java:452)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.parse.type.HiveFunctionHelper.createObjectInspector(HiveFunctionHelper.java:435)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.parse.type.HiveFunctionHelper.getReturnType(HiveFunctionHelper.java:124)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.parse.type.RexNodeExprFactory.createFuncCallExpr(RexNodeExprFactory.java:647)
 ~[hive-exec-4.0.0-SNAPSHOT.jar:4.0.0-SNAPSHOT]
{code}



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

Re: Open old PRs

[Zoltan Haindrich]

On 6/2/20 3:10 PM, David Mollitor wrote:

I think we might want to take one manual pass across the board.  It will
most likely take more than 7 days to get through them all, so it may be
closing things that are legitimate.


yeah...a manual pass would be good; I went thru around 10 or so before I've 
wrote the first mail in this thread...
and I definetly don't want to go thru 400 - so I would preffer the bot :D



One low hanging fruit (that applied to one of my PRs).  The JIRA it was
associated with was already closed.  Is there a way to target those?


yes; there might be certainly a lot of those...(that's why I've estimate to 1/3 
to be applicable)
but filtering out even this is an awful lot of work (or it might involve writing a 
"bot")...
if it's important enough the contributor could reopen / rebase the patch.
We could try to communicate the non-hostaile intention in the message placed by 
the bot.
The current message is the stale PRs would get is:
"This pull request has been automatically marked as stale because it has not had 
recent activity. It will be closed if no further activity occurs."


Also, I have submitted my first PR to test out the new system.  It
has passed tests.  Ashutoshc has generously provided a +1.  What's the
next step to get it merged into the master?  Do I download the patch from
Github and apply manually using my Apache credentials?  Is the "merge"
feature setup in Github?  As I understand it, GitHub is only mirroring the
Apache git system.  Whatever the process we need an update in the
HowToContribute docs.


That's an interesting question; the github repo is linked to the apache repo - 
so you may push/merge/whatever on the github interface; it will work.
Github supports 3 modes to merge PRs:
* We should definetly disable the "merge" option as that will just create a 
internation railways station from our history :)
* rebase doesn't make it easier for reviewier to keep track new 
changes...because the PR owner have to continuosly force push the branch
* squash merge work great - and I remembered that it changes the author to the user pushing the "squash" button; however right now it seems that it changes the author to 
the "user who opened the pr" which looks good-enough for me!

(I've added the neccessary .asf.yaml changes to the existing PR)

cheers,
Zoltan


https://github.com/apache/hive/pull/1045
https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-ApplyingaPatch


Thanks!

On Tue, Jun 2, 2020 at 4:58 AM Zoltan Haindrich  wrote:


I think to use "probot" we would need to ask infra to configure the
"probot" github app.
It seems to me that the stale plugin from github actions provides almost
the same functionaluty - as they seem to be more or less identical in
features I would go with the
latter.

I've opened a pr to enable stale on the hive repo:
by default it will mark as stale afte 60 days; and close it if there is no
activity for another 7 days
https://github.com/apache/hive/pull/1049

cheers,
Zoltan


On 6/2/20 5:00 AM, Ashutosh Chauhan wrote:

How about using stalebot : https://github.com/probot/stale ?

On Mon, Jun 1, 2020 at 12:20 PM Zoltán Haindrich  wrote:




Hey David,

On June 1, 2020 3:52:05 PM GMT+02:00, David Mollitor 


wrote:

Any idea how long it will take to run precomit on all existing PRs?


I'm not entirely sure, but a rough estimate could be:
* not every pr is mergeable; there are many which was already
merged/outdated/etc...lets estimate that 1/3 is mergeable
* every pr runs for at least 1 hours

this would mean 430/3*1/24 days of test execution which is at least 6

days.


I see little to no value in running tests on archaic prs.
We could also configure an automatism to close prs after some time of
inactivity
https://github.com/actions/stale/blob/master/README.md
The good side of this is that it will get rid of ancient prs; however it
might seem rude to a contributor in case he is waiting for feedback or
something
Even with that argument I think we should configure it at least for a

few

days to get rid of the dangling prs of almost a decade ! (pr#2 is

opened in

2011)...
What do you think?

cheers,
Zoltan



On Mon, Jun 1, 2020 at 9:49 AM Panos Garefalakis 
wrote:


Same here, however, there are still ~ 430 PRs pending on master.
Thanks Zoltan for this great initiative!

Cheers,
Panagiotis

On Mon, Jun 1, 2020 at 2:33 PM David Mollitor 

wrote:



Thanks so much for the work on this.

Just cleaned up mine.

On Sat, May 30, 2020 at 10:16 AM Zoltan Haindrich 

wrote:



Hey All,

The new test executor will pick up any PR which doesn't yet have

a test

result - now that the patch is on the master; every PR which is

mergeable

with the master branch is
a good candidate - so the right move would be to clean up our PR

backlog.


I would like to ask everyone to look at
https://github.com/apache/hive/pulls
and close some PRs which are already submitted or just leftovers

>from -

primarily I would ask you to lo

[jira] [Created] (HIVE-23592) BuddyAllocator "makeIntPair" is Not Correct

[David Mollitor (Jira)]

David Mollitor created HIVE-23592:
-

 Summary: BuddyAllocator "makeIntPair" is Not Correct
 Key: HIVE-23592
 URL: https://issues.apache.org/jira/browse/HIVE-23592
 Project: Hive
  Issue Type: Bug
  Components: llap
Affects Versions: 3.2.0
Reporter: David Mollitor
Assignee: David Mollitor


{code:java|title=BuddyAllocator.java}
  // Utility methods used to store pairs of ints as long.
  private static long makeIntPair(int first, int second) {
return ((long)first) << 32 | second;
  }
  private static int getFirstInt(long result) {
return (int) (result >>> 32);
  }
  private static int getSecondInt(long result) {
return (int) (result & ((1L << 32) - 1));
  }
{code}
{code:java}
long result = LLAP.makeIntPair(Integer.MIN_VALUE, Integer.MIN_VALUE);
if (LLAP.getFirstInt(result) != Integer.MIN_VALUE) {
   throw new Exception();
}
if (LLAP.getSecondInt(result) != Integer.MIN_VALUE) {
  throw new Exception();
   }
/*
 * Exception in thread "main" java.lang.Exception
 *  at org.test.TestMe.main(TestMe.java:19)
 */
{code}
[https://github.com/apache/hive/blob/4b670877c280b37c5776046f66d766079489b2a8/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java#L1677]



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

Re: Open old PRs

[David Mollitor]

Hey,

I think we might want to take one manual pass across the board.  It will
most likely take more than 7 days to get through them all, so it may be
closing things that are legitimate.

One low hanging fruit (that applied to one of my PRs).  The JIRA it was
associated with was already closed.  Is there a way to target those?

Also, I have submitted my first PR to test out the new system.  It
has passed tests.  Ashutoshc has generously provided a +1.  What's the
next step to get it merged into the master?  Do I download the patch from
Github and apply manually using my Apache credentials?  Is the "merge"
feature setup in Github?  As I understand it, GitHub is only mirroring the
Apache git system.  Whatever the process we need an update in the
HowToContribute docs.

https://github.com/apache/hive/pull/1045
https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-ApplyingaPatch


Thanks!

On Tue, Jun 2, 2020 at 4:58 AM Zoltan Haindrich  wrote:

> I think to use "probot" we would need to ask infra to configure the
> "probot" github app.
> It seems to me that the stale plugin from github actions provides almost
> the same functionaluty - as they seem to be more or less identical in
> features I would go with the
> latter.
>
> I've opened a pr to enable stale on the hive repo:
> by default it will mark as stale afte 60 days; and close it if there is no
> activity for another 7 days
> https://github.com/apache/hive/pull/1049
>
> cheers,
> Zoltan
>
>
> On 6/2/20 5:00 AM, Ashutosh Chauhan wrote:
> > How about using stalebot : https://github.com/probot/stale ?
> >
> > On Mon, Jun 1, 2020 at 12:20 PM Zoltán Haindrich  wrote:
> >
> >>
> >>
> >> Hey David,
> >>
> >> On June 1, 2020 3:52:05 PM GMT+02:00, David Mollitor  >
> >> wrote:
> >>> Any idea how long it will take to run precomit on all existing PRs?
> >>>
> >> I'm not entirely sure, but a rough estimate could be:
> >> * not every pr is mergeable; there are many which was already
> >> merged/outdated/etc...lets estimate that 1/3 is mergeable
> >> * every pr runs for at least 1 hours
> >>
> >> this would mean 430/3*1/24 days of test execution which is at least 6
> days.
> >>
> >> I see little to no value in running tests on archaic prs.
> >> We could also configure an automatism to close prs after some time of
> >> inactivity
> >> https://github.com/actions/stale/blob/master/README.md
> >> The good side of this is that it will get rid of ancient prs; however it
> >> might seem rude to a contributor in case he is waiting for feedback or
> >> something
> >> Even with that argument I think we should configure it at least for a
> few
> >> days to get rid of the dangling prs of almost a decade ! (pr#2 is
> opened in
> >> 2011)...
> >> What do you think?
> >>
> >> cheers,
> >> Zoltan
> >>
> >>
> >>> On Mon, Jun 1, 2020 at 9:49 AM Panos Garefalakis 
> >>> wrote:
> >>>
>  Same here, however, there are still ~ 430 PRs pending on master.
>  Thanks Zoltan for this great initiative!
> 
>  Cheers,
>  Panagiotis
> 
>  On Mon, Jun 1, 2020 at 2:33 PM David Mollitor 
> >>> wrote:
> 
> > Thanks so much for the work on this.
> >
> > Just cleaned up mine.
> >
> > On Sat, May 30, 2020 at 10:16 AM Zoltan Haindrich 
> >>> wrote:
> >
> >> Hey All,
> >>
> >> The new test executor will pick up any PR which doesn't yet have
> >>> a test
> >> result - now that the patch is on the master; every PR which is
>  mergeable
> >> with the master branch is
> >> a good candidate - so the right move would be to clean up our PR
>  backlog.
> >>
> >> I would like to ask everyone to look at
> >> https://github.com/apache/hive/pulls
> >> and close some PRs which are already submitted or just leftovers
> >> >from -
> >> primarily I would ask you to look at PRs opened by yourself...
> >>
> >> cheers,
> >> Zoltan
> >>
> >
> 
> >>
> >> --
> >> Zoltán Haindrich
> >>
> >
>

Re: [DISCUSS] Replace ptest with hive-test-kube

[Zoltan Haindrich]

Hello,

I would like to note that you may login to the jenkins instance - to start/kill 
builds (or create new jobs).
I've configured github oauth - but since team membership can't be queried from the "apache 
organization" - it's harder to configure all "hive committers".
However...I think I've made it available for most of us - if you can't start 
builds/etc just let me know your github user and I'll add it.

cheers,
Zoltan



On 5/29/20 2:32 PM, Zoltan Haindrich wrote:

Hey all!

The patch is now in master - so every new PR or a push on it will trigger a new 
run.

Please decide which one would you like to use - open a PR to see the new one work...or upload a patch file to the jira - but please don't do both; because in that case 2 
execution will happen.


The job execution time(2-4 hours) of a single run is a bit higher than the 
usual on the ptest server - this is mostly to increase throughput.

The patch also disabled a set of tests; I will send the full list of skipped 
tests shortly.

cheers,
Zoltan


On 5/27/20 1:50 PM, Zoltan Haindrich wrote:

Hello all!

The new stuff is ready to be switched on-to. It needs to be merged into master 
- and after that anyone who opens a PR will get a run by the new HiveQA infra.
I propose to run the 2 systems side-by-side for some time - the regular master 
builds will start; and we will see how frequently that is polluted by flaky 
tests.

Note that the current patch also disables around ~25 more tests to increase stability - to get a better overview about the disabled tests I think the "direction of the 
information flow" should be altered; what I mean by that is: instead of just throwing in a jira for "disable test x" and opening a new one like "fix test x"; only open 
the latter and place the jira reference into the ignore message; meanwhile also add a regular report about the actually disabled tests - so people who do know about the 
importance of a particular test can get involved.


Note: the builds.apache.org instance will be shutdown somewhere in the future as well...but I think the new one is a good-enough alternative to not have to migrate the 
Hive-precommit job over to https://ci-hadoop.apache.org/.


http://34.66.156.144:8080/job/hive-precommit/job/PR-948/5/
https://issues.apache.org/jira/browse/HIVE-22942
https://github.com/apache/hive/pull/948/files

cheers,
Zoltan

On 5/18/20 1:42 PM, Zoltan Haindrich wrote:

Hey!

On 5/18/20 11:51 AM, Zoltan Chovan wrote:

Thank you for all of your efforts, this looks really promising. With moving
to github PRs, would that also mean that we move away from the reviewboard
for code review?
I didn't thinked about that. I think using github's review interface will remain optional, because both review systems has there own strong points - I wouldn't force 
anyone to use one over the other. (For some patches reviewboard is much better; because it's able to track content moves a bit better than github. - meanwhile github has 
a small feature that enables to mark files as reviewed)
As a matter of fact we had sometimes patches on the jira's which never had neither an RB or a PR to review them - having a PR there at least will make it easier for 
reviewers to comment.



Also, what happens if a PR is updated? Will the tests run for both or just
for the latest version?
It will trigger a new build - if there is already a build in progress that will prevent a new build from starting until it finishes...and there is also a 5 builds/day 
limit; which might induce some wait.


cheers,
Zoltan



Regards,
Zoltan

On Sun, May 17, 2020 at 10:51 PM Zoltan Haindrich  wrote:


Hello all!

The proposed system have become more stable lately - and I think I've
solved a few sources of flakiness.
To be really usable I also wanted to add a way to dynamically
enable/disable a set of tests (for example the replication tests take ~7
hours to execute from the total of 24
hours - and they are also a bit unstable, so not running them when not
neccesary would be beneficial in multiple ways) - but to do this the best
would be to throw in
junit5; unfortunately the current ptest installation uses maven 3.0.5
which doesn't like these kind of things - so instead of hacking a fix for
that I've removed it
from the dev branch for now.

I would like to propose to start an evaluation phase of the new test
procedures(INFRA-20269)
The process would look something like this:
* someone opens a PR - the tests will be run on the changes
* on every active branches the tests will run from time to time
    * this will produce a bunch of test runs on the master branch as well ;
which will show how well the tests behave on the master branch without any
patches
* runs on branches (PRs or active development branches(eg:master)) will be
rate limited to 5 builds/day
* at most ~4 builds at a time - to maximize resource usage
* turnaround time for a build is right now 2 hours - which I feel like a
balanced choice between speed/response time

Possible future benefits:
*

[jira] [Created] (HIVE-23591) When Worker fails to connect to metastore it should wait before retrying

[Peter Vary (Jira)]

Peter Vary created HIVE-23591:
-

 Summary: When Worker fails to connect to metastore it should wait 
before retrying
 Key: HIVE-23591
 URL: https://issues.apache.org/jira/browse/HIVE-23591
 Project: Hive
  Issue Type: Bug
Reporter: Peter Vary
Assignee: Peter Vary


HIVE-23555 removed the sleep after Thrift exception. This would result in an 
active loop if the HMS is unreachable.
We should wait before trying to connect again to the HMS.



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

Re: Open old PRs

[Zoltan Haindrich]

I think to use "probot" we would need to ask infra to configure the "probot" 
github app.
It seems to me that the stale plugin from github actions provides almost the same functionaluty - as they seem to be more or less identical in features I would go with the 
latter.


I've opened a pr to enable stale on the hive repo:
by default it will mark as stale afte 60 days; and close it if there is no 
activity for another 7 days
https://github.com/apache/hive/pull/1049

cheers,
Zoltan


On 6/2/20 5:00 AM, Ashutosh Chauhan wrote:

How about using stalebot : https://github.com/probot/stale ?

On Mon, Jun 1, 2020 at 12:20 PM Zoltán Haindrich  wrote:




Hey David,

On June 1, 2020 3:52:05 PM GMT+02:00, David Mollitor 
wrote:

Any idea how long it will take to run precomit on all existing PRs?


I'm not entirely sure, but a rough estimate could be:
* not every pr is mergeable; there are many which was already
merged/outdated/etc...lets estimate that 1/3 is mergeable
* every pr runs for at least 1 hours

this would mean 430/3*1/24 days of test execution which is at least 6 days.

I see little to no value in running tests on archaic prs.
We could also configure an automatism to close prs after some time of
inactivity
https://github.com/actions/stale/blob/master/README.md
The good side of this is that it will get rid of ancient prs; however it
might seem rude to a contributor in case he is waiting for feedback or
something
Even with that argument I think we should configure it at least for a few
days to get rid of the dangling prs of almost a decade ! (pr#2 is opened in
2011)...
What do you think?

cheers,
Zoltan



On Mon, Jun 1, 2020 at 9:49 AM Panos Garefalakis 
wrote:


Same here, however, there are still ~ 430 PRs pending on master.
Thanks Zoltan for this great initiative!

Cheers,
Panagiotis

On Mon, Jun 1, 2020 at 2:33 PM David Mollitor 

wrote:



Thanks so much for the work on this.

Just cleaned up mine.

On Sat, May 30, 2020 at 10:16 AM Zoltan Haindrich 

wrote:



Hey All,

The new test executor will pick up any PR which doesn't yet have

a test

result - now that the patch is on the master; every PR which is

mergeable

with the master branch is
a good candidate - so the right move would be to clean up our PR

backlog.


I would like to ask everyone to look at
https://github.com/apache/hive/pulls
and close some PRs which are already submitted or just leftovers

>from -

primarily I would ask you to look at PRs opened by yourself...

cheers,
Zoltan







--
Zoltán Haindrich

[jira] [Created] (HIVE-23590) Close stale PRs automatically

[Zoltan Haindrich (Jira)]

Zoltan Haindrich created HIVE-23590:
---

 Summary: Close stale PRs automatically
 Key: HIVE-23590
 URL: https://issues.apache.org/jira/browse/HIVE-23590
 Project: Hive
  Issue Type: Improvement
Reporter: Zoltan Haindrich
Assignee: Zoltan Haindrich






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