Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Laszlo Pinter via Review Board]

> On Dec. 4, 2018, 10:08 p.m., Bharathkrishna Guruvayoor Murali wrote:
> > Hi, the patch looks good, I just had a quick question. Does this mean that 
> > all the alters happen in one transaction? Will it prevent concurrent 
> > operations for the whole time a large number of partitions are altered?

No it will not. What was changed in this patch, that the list of partitions are 
passed to the metastore in one call, keeping the opened connections to minimum. 
Under the hood everything stayed the same, so it is possible to perform 
concurrent operations.


- Laszlo


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


On Nov. 30, 2018, 11:31 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 30, 2018, 11:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Bharathkrishna Guruvayoor Murali via Review Board]

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



Hi, the patch looks good, I just had a quick question. Does this mean that all 
the alters happen in one transaction? Will it prevent concurrent operations for 
the whole time a large number of partitions are altered?

- Bharathkrishna Guruvayoor Murali


On Nov. 30, 2018, 11:31 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 30, 2018, 11:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Laszlo Pinter via Review Board]

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

(Updated Nov. 30, 2018, 11:31 a.m.)


Review request for hive.


Repository: hive-git


Description
---

HIVE-20891: Call alter_partition in batch when dynamically loading partitions


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
e8f362357537e73502f743a9df189dec9be2da5d 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
e185bf49d42da9d1497643c20bbd71edaf071bf1 


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

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


Testing
---


Thanks,

Laszlo Pinter

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Peter Vary via Review Board]

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



Mostly just questions about logging


ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java
Lines 139 (patched)


nit: Might want to remove this extra line



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2087 (patched)


Does this really worth a change?
I usually use this only if generating the log message is costly. The 
LOG.debug will definitely will start with the same check...



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2161 (patched)


Does this really worth a change?
I usually use this only if generating the log message is costly. The 
LOG.debug will definitely will start with the same check...



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2211 (patched)


Does this really worth a change?
I usually use this only if generating the log message is costly. The 
LOG.debug will definitely will start with the same check...



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2345 (patched)


Is the output readable with multiple partitions?



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2659 (patched)


Does this really worth a change?
I usually use this only if generating the log message is costly. The 
LOG.debug will definitely will start with the same check...



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2720 (patched)


Does this really worth a change?
I usually use this only if generating the log message is costly. The 
LOG.debug will definitely will start with the same check...


- Peter Vary


On nov. 15, 2018, 8:52 de, Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated nov. 15, 2018, 8:52 de)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Denys Kuzmenko via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2350 (patched)


You might consider using a custom thread pool.


- Denys Kuzmenko


On Nov. 15, 2018, 8:52 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 15, 2018, 8:52 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Denys Kuzmenko via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Line 2310 (original), 2319 (patched)


This method could reuse new partitions method to avoid code duplication.


- Denys Kuzmenko


On Nov. 15, 2018, 8:52 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 15, 2018, 8:52 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Denys Kuzmenko via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2350 (patched)


I would extract result to liocal variable for better readability.


- Denys Kuzmenko


On Nov. 15, 2018, 8:52 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 15, 2018, 8:52 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Denys Kuzmenko via Review Board]

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




ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java
Lines 90 (patched)


3 first method params could be grouped into Table object.


- Denys Kuzmenko


On Nov. 15, 2018, 8:52 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 15, 2018, 8:52 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Re: Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Denys Kuzmenko via Review Board]

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2692 (patched)


By default parallelStream uses the ForkJoinPool.commonPool(), a Thread Pool 
shared by the entire application. You might consider creating a custom one.


- Denys Kuzmenko


On Nov. 15, 2018, 8:52 a.m., Laszlo Pinter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69341/
> ---
> 
> (Updated Nov. 15, 2018, 8:52 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20891: Call alter_partition in batch when dynamically loading partitions
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
> e8f362357537e73502f743a9df189dec9be2da5d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e185bf49d42da9d1497643c20bbd71edaf071bf1 
> 
> 
> Diff: https://reviews.apache.org/r/69341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Laszlo Pinter
> 
>

Review Request 69341: HIVE-20891: Call alter_partition in batch when dynamically loading partitions

[Laszlo Pinter via Review Board]

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

Review request for hive.


Repository: hive-git


Description
---

HIVE-20891: Call alter_partition in batch when dynamically loading partitions


Diffs
-

  ql/src/java/org/apache/hadoop/hive/metastore/SynchronizedMetaStoreClient.java 
e8f362357537e73502f743a9df189dec9be2da5d 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
e185bf49d42da9d1497643c20bbd71edaf071bf1 


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


Testing
---


Thanks,

Laszlo Pinter