You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jason Dere <jd...@hortonworks.com> on 2018/03/22 00:16:15 UTC
Review Request 66204: HIVE-19017: Add util function to determine if 2
ValidWriteIdLists are at the same committed ID
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66204/
-----------------------------------------------------------
Review request for hive, Eugene Koifman, Gopal V, and Jesús Camacho Rodríguez.
Bugs: HIVE-19017
https://issues.apache.org/jira/browse/HIVE-19017
Repository: hive-git
Description
-------
Create utility comparison function and some simple tests.
Diffs
-----
storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java PRE-CREATION
storage-api/src/test/org/apache/hive/common/util/TestTxnIdUtils.java PRE-CREATION
Diff: https://reviews.apache.org/r/66204/diff/1/
Testing
-------
Thanks,
Jason Dere
Re: Review Request 66204: HIVE-19017: Add util function to determine
if 2 ValidWriteIdLists are at the same committed ID
Posted by Jason Dere <jd...@hortonworks.com>.
> On March 22, 2018, 12:26 a.m., Jesús Camacho Rodríguez wrote:
> > storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/66204/diff/1/?file=1985166#file1985166line41>
> >
> > Can minOpenId be null for both lists? What's the semantics then?
Ok, looking at TxnUtils.createValidReaderWriteIdList(), it looks like it could potentially be null in the case that there is no min open ID (they are all committed?). Eugene does that sound right?
In which case, if both are in that condition then I suspect they would be at the same commit point only if they have the same highwater mark (if they have different highwater marks then one of them has more committed IDs than the other).
I'll update the logic and tests.
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66204/#review199731
-----------------------------------------------------------
On March 22, 2018, 12:16 a.m., Jason Dere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66204/
> -----------------------------------------------------------
>
> (Updated March 22, 2018, 12:16 a.m.)
>
>
> Review request for hive, Eugene Koifman, Gopal V, and Jesús Camacho Rodríguez.
>
>
> Bugs: HIVE-19017
> https://issues.apache.org/jira/browse/HIVE-19017
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create utility comparison function and some simple tests.
>
>
> Diffs
> -----
>
> storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java PRE-CREATION
> storage-api/src/test/org/apache/hive/common/util/TestTxnIdUtils.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/66204/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Dere
>
>
Re: Review Request 66204: HIVE-19017: Add util function to determine
if 2 ValidWriteIdLists are at the same committed ID
Posted by Jason Dere <jd...@hortonworks.com>.
> On March 22, 2018, 12:26 a.m., Jesús Camacho Rodríguez wrote:
> > storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/66204/diff/1/?file=1985166#file1985166line41>
> >
> > Can minOpenId be null for both lists? What's the semantics then?
>
> Jason Dere wrote:
> Ok, looking at TxnUtils.createValidReaderWriteIdList(), it looks like it could potentially be null in the case that there is no min open ID (they are all committed?). Eugene does that sound right?
> In which case, if both are in that condition then I suspect they would be at the same commit point only if they have the same highwater mark (if they have different highwater marks then one of them has more committed IDs than the other).
> I'll update the logic and tests.
Spoke to Eugene and looked at this again, actually I don't think the minOpenID is needed at all for this check, just the highwater mark and the invalid IDs list. I've removed the minOpenID checking, and the rest of the checks are unchanged. Also added some additional test cases.
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66204/#review199731
-----------------------------------------------------------
On March 22, 2018, 12:16 a.m., Jason Dere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66204/
> -----------------------------------------------------------
>
> (Updated March 22, 2018, 12:16 a.m.)
>
>
> Review request for hive, Eugene Koifman, Gopal V, and Jesús Camacho Rodríguez.
>
>
> Bugs: HIVE-19017
> https://issues.apache.org/jira/browse/HIVE-19017
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create utility comparison function and some simple tests.
>
>
> Diffs
> -----
>
> storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java PRE-CREATION
> storage-api/src/test/org/apache/hive/common/util/TestTxnIdUtils.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/66204/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Dere
>
>
Re: Review Request 66204: HIVE-19017: Add util function to determine
if 2 ValidWriteIdLists are at the same committed ID
Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
> On March 22, 2018, 12:26 a.m., Jesús Camacho Rodríguez wrote:
> > storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/66204/diff/1/?file=1985166#file1985166line41>
> >
> > Can minOpenId be null for both lists? What's the semantics then?
>
> Jason Dere wrote:
> Ok, looking at TxnUtils.createValidReaderWriteIdList(), it looks like it could potentially be null in the case that there is no min open ID (they are all committed?). Eugene does that sound right?
> In which case, if both are in that condition then I suspect they would be at the same commit point only if they have the same highwater mark (if they have different highwater marks then one of them has more committed IDs than the other).
> I'll update the logic and tests.
>
> Jason Dere wrote:
> Spoke to Eugene and looked at this again, actually I don't think the minOpenID is needed at all for this check, just the highwater mark and the invalid IDs list. I've removed the minOpenID checking, and the rest of the checks are unchanged. Also added some additional test cases.
LGTM.
- Jesús
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66204/#review199731
-----------------------------------------------------------
On March 23, 2018, 5:25 p.m., Jason Dere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66204/
> -----------------------------------------------------------
>
> (Updated March 23, 2018, 5:25 p.m.)
>
>
> Review request for hive, Eugene Koifman, Gopal V, and Jesús Camacho Rodríguez.
>
>
> Bugs: HIVE-19017
> https://issues.apache.org/jira/browse/HIVE-19017
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create utility comparison function and some simple tests.
>
>
> Diffs
> -----
>
> storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java PRE-CREATION
> storage-api/src/test/org/apache/hive/common/util/TestTxnIdUtils.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/66204/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Dere
>
>
Re: Review Request 66204: HIVE-19017: Add util function to determine
if 2 ValidWriteIdLists are at the same committed ID
Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66204/#review199731
-----------------------------------------------------------
storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java
Lines 41 (patched)
<https://reviews.apache.org/r/66204/#comment280169>
Can minOpenId be null for both lists? What's the semantics then?
- Jesús Camacho Rodríguez
On March 22, 2018, 12:16 a.m., Jason Dere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66204/
> -----------------------------------------------------------
>
> (Updated March 22, 2018, 12:16 a.m.)
>
>
> Review request for hive, Eugene Koifman, Gopal V, and Jesús Camacho Rodríguez.
>
>
> Bugs: HIVE-19017
> https://issues.apache.org/jira/browse/HIVE-19017
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create utility comparison function and some simple tests.
>
>
> Diffs
> -----
>
> storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java PRE-CREATION
> storage-api/src/test/org/apache/hive/common/util/TestTxnIdUtils.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/66204/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Dere
>
>
Re: Review Request 66204: HIVE-19017: Add util function to determine
if 2 ValidWriteIdLists are at the same committed ID
Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66204/
-----------------------------------------------------------
(Updated March 23, 2018, 5:25 p.m.)
Review request for hive, Eugene Koifman, Gopal V, and Jesús Camacho Rodríguez.
Changes
-------
Removed MinOpenID checking, not necessary.
Bugs: HIVE-19017
https://issues.apache.org/jira/browse/HIVE-19017
Repository: hive-git
Description
-------
Create utility comparison function and some simple tests.
Diffs (updated)
-----
storage-api/src/java/org/apache/hive/common/util/TxnIdUtils.java PRE-CREATION
storage-api/src/test/org/apache/hive/common/util/TestTxnIdUtils.java PRE-CREATION
Diff: https://reviews.apache.org/r/66204/diff/2/
Changes: https://reviews.apache.org/r/66204/diff/1-2/
Testing
-------
Thanks,
Jason Dere