You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Laszlo Pinter via Review Board <no...@reviews.apache.org> on 2018/11/15 08:52:09 UTC
Review Request 69341: HIVE-20891: Call alter_partition in batch when
dynamically loading partitions
-----------------------------------------------------------
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
Re: Review Request 69341: HIVE-20891: Call alter_partition in batch
when dynamically loading partitions
Posted by Laszlo Pinter via Review Board <no...@reviews.apache.org>.
> 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
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Laszlo Pinter via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Denys Kuzmenko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69341/#comment295271>
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
Posted by Denys Kuzmenko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69341/#comment295269>
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
>
>
Re: Review Request 69341: HIVE-20891: Call alter_partition in batch
when dynamically loading partitions
Posted by Denys Kuzmenko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69341/#comment295270>
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
Posted by Denys Kuzmenko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69341/#comment295273>
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
Posted by Denys Kuzmenko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69341/#comment295272>
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
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/69341/#comment295503>
nit: Might want to remove this extra line
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2087 (patched)
<https://reviews.apache.org/r/69341/#comment295504>
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)
<https://reviews.apache.org/r/69341/#comment295502>
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)
<https://reviews.apache.org/r/69341/#comment295505>
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)
<https://reviews.apache.org/r/69341/#comment295508>
Is the output readable with multiple partitions?
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 2659 (patched)
<https://reviews.apache.org/r/69341/#comment295506>
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)
<https://reviews.apache.org/r/69341/#comment295507>
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
>
>