You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Barna Zsombor Klara <zs...@cloudera.com> on 2016/08/24 18:06:58 UTC
Review Request 51381: HIVE-14427: CompactionTxnHandler.markCleaned()
can delete aborted txns
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51381/
-----------------------------------------------------------
Review request for hive and Eugene Koifman.
Repository: hive-git
Description
-------
HIVE-14427: CompactionTxnHandler.markCleaned() can delete aborted txns
Diffs
-----
metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 75a4d875562f9ec85d2d02e36526de8b1e0c4404
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java f513d0fa791f24da4ce6312df9923db692ef7129
Diff: https://reviews.apache.org/r/51381/diff/
Testing
-------
Thanks,
Barna Zsombor Klara
Re: Review Request 51381: HIVE-14427:
CompactionTxnHandler.markCleaned() can delete aborted txns
Posted by Barna Zsombor Klara <zs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51381/
-----------------------------------------------------------
(Updated Aug. 26, 2016, 12:12 a.m.)
Review request for hive and Eugene Koifman.
Changes
-------
Changed the string concatenation to StringBuilder.append where possible and removed the unneeded append("");
Repository: hive-git
Description
-------
HIVE-14427: CompactionTxnHandler.markCleaned() can delete aborted txns
Diffs (updated)
-----
metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 75a4d875562f9ec85d2d02e36526de8b1e0c4404
ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java f513d0fa791f24da4ce6312df9923db692ef7129
Diff: https://reviews.apache.org/r/51381/diff/
Testing
-------
Thanks,
Barna Zsombor Klara
Re: Review Request 51381: HIVE-14427:
CompactionTxnHandler.markCleaned() can delete aborted txns
Posted by Barna Zsombor Klara <zs...@cloudera.com>.
> On Aug. 25, 2016, 11:47 p.m., Gabor Szadovszky wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java, line 412
> > <https://reviews.apache.org/r/51381/diff/1/?file=1483999#file1483999line412>
> >
> > Not sure what would be the maximum number of rows. Is int enough?
At first I thought that Integer should be enough to hold the number of transactions on a table/partition, but I'll defer to the better judgement of anyone with more experience in the transaction compaction implementation.
> On Aug. 25, 2016, 11:47 p.m., Gabor Szadovszky wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java, line 384
> > <https://reviews.apache.org/r/51381/diff/1/?file=1483999#file1483999line384>
> >
> > Is an int enough for this sum of ints?
At first I thought that Integer should be enough to hold the number of transactions on a table/partition, but I'll defer to the better judgement of anyone with more experience in the transaction compaction implementation.
- Barna Zsombor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51381/#review146886
-----------------------------------------------------------
On Aug. 26, 2016, 12:12 a.m., Barna Zsombor Klara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51381/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2016, 12:12 a.m.)
>
>
> Review request for hive and Eugene Koifman.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-14427: CompactionTxnHandler.markCleaned() can delete aborted txns
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 75a4d875562f9ec85d2d02e36526de8b1e0c4404
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java f513d0fa791f24da4ce6312df9923db692ef7129
>
> Diff: https://reviews.apache.org/r/51381/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Barna Zsombor Klara
>
>
Re: Review Request 51381: HIVE-14427:
CompactionTxnHandler.markCleaned() can delete aborted txns
Posted by Gabor Szadovszky <ga...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51381/#review146886
-----------------------------------------------------------
Some minor findings/questions. LGTM, otherwise.
metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java (line 370)
<https://reviews.apache.org/r/51381/#comment213661>
As you are already using StringBuilder, I would suggest using .append(...) instead of '+'.
metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java (line 384)
<https://reviews.apache.org/r/51381/#comment213665>
Is an int enough for this sum of ints?
metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java (line 412)
<https://reviews.apache.org/r/51381/#comment213666>
Not sure what would be the maximum number of rows. Is int enough?
metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java (line 501)
<https://reviews.apache.org/r/51381/#comment213669>
Is there a missing string to be added? I don't think it make sense to add an empty one...
- Gabor Szadovszky
On Aug. 24, 2016, 6:06 p.m., Barna Zsombor Klara wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51381/
> -----------------------------------------------------------
>
> (Updated Aug. 24, 2016, 6:06 p.m.)
>
>
> Review request for hive and Eugene Koifman.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-14427: CompactionTxnHandler.markCleaned() can delete aborted txns
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java 75a4d875562f9ec85d2d02e36526de8b1e0c4404
> ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java f513d0fa791f24da4ce6312df9923db692ef7129
>
> Diff: https://reviews.apache.org/r/51381/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Barna Zsombor Klara
>
>