You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by denys kuzmenko via Review Board <no...@reviews.apache.org> on 2018/08/24 07:28:32 UTC

Review Request 68496: Optimized & cleaned up HBaseQTest runner

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

Review request for hive, Marta Kuczora and Peter Vary.


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


Repository: hive-git


Description
-------

1. Set proper cluster destroy order
2. Propagated proper HBaseTestContext
3. Ported downstream fixes (CDH-63695)
4. General clean up


Diffs
-----

  hbase-handler/src/test/queries/negative/cascade_dbdrop.q 48be8cd07018d005a28ad8070e13c51a46cf6d06 
  hbase-handler/src/test/queries/positive/hbase_handler_snapshot.q e4290717a85a462a45bc14e6295fa7eccad8a51d 
  hbase-handler/src/test/results/negative/cascade_dbdrop.q.out 803e35e40681df017a2f4e6d017b72749417d461 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestLocationQueries.java 31195c4523a35caf473eeecd8c8142cd1265ca18 
  itests/util/src/main/java/org/apache/hadoop/hive/accumulo/AccumuloQTestUtil.java 956478d778be03070b242ae45d340b5a99ac9316 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCoreBlobstoreCliDriver.java 3cf5ebb3df3455f9ebe701b79f5814e40f34ccc5 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCliDriver.java 1ead1448d1e1a981068c4bdf5484346babb72ec0 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCompareCliDriver.java 6b4c6c6a794da754bcaf8c4374fd9f85d51f318f 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java 70cbf04823bb3d7208df245074eb9b732eadd18e 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java c76a70e7dddd2c13c2b42c32526398c639f041e3 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreNegativeCliDriver.java 07ae6ac206926c450c86b9519da469fc51f1d55d 
  itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CorePerfCliDriver.java 55e744e0f3d7b266593f4c8fb82eb8539ac0a563 
  itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseQTestUtil.java 07df0c9d1ed7fa3bf28325a1c37acf043d8ca848 
  itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseTestSetup.java 7b203a928110101ba0de695acacadef1f1484b44 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestArguments.java PRE-CREATION 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 5adbb63693b1f2fafa47939b2833310f5dd96bf2 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/parse/CoreParseNegative.java 8f5744d2f1c8c059b4148aafde0558d76b723b65 


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


Testing
-------

fixed existing tests


Thanks,

denys kuzmenko


Re: Review Request 68496: Optimized & cleaned up HBaseQTest runner

Posted by denys kuzmenko via Review Board <no...@reviews.apache.org>.

> On Aug. 24, 2018, 8:17 a.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
> > Line 82 (original), 82 (patched)
> > <https://reviews.apache.org/r/68496/diff/1/?file=2077050#file2077050line84>
> >
> >     Since we removed clearTestSideEffects here, are we sure that clearPostTestEffects does the same removal?

This is how it was initially before the session lifecycle change (that is unrelated to cleanup, same way it's done downstream):
HIVE-19882: Fix QTestUtil session lifecycle (Zoltan Haindrich reviewed by Jason Dere) Zoltan Haindrich 2018. 06. 21. 6:15


> On Aug. 24, 2018, 8:17 a.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java
> > Line 78 (original), 77 (patched)
> > <https://reviews.apache.org/r/68496/diff/1/?file=2077051#file2077051line79>
> >
> >     Same as above. Does clearPostEffects is enough?

This is how it was initially before the session lifecycle change (that is unrelated to cleanup, same way it's done downstream):
HIVE-19882: Fix QTestUtil session lifecycle (Zoltan Haindrich reviewed by Jason Dere) Zoltan Haindrich 2018. 06. 21. 6:15


- denys


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


On Aug. 24, 2018, 7:28 a.m., denys kuzmenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68496/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2018, 7:28 a.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-20394
>     https://issues.apache.org/jira/browse/HIVE-20394
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> 1. Set proper cluster destroy order
> 2. Propagated proper HBaseTestContext
> 3. Ported downstream fixes (CDH-63695)
> 4. General clean up
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/test/queries/negative/cascade_dbdrop.q 48be8cd07018d005a28ad8070e13c51a46cf6d06 
>   hbase-handler/src/test/queries/positive/hbase_handler_snapshot.q e4290717a85a462a45bc14e6295fa7eccad8a51d 
>   hbase-handler/src/test/results/negative/cascade_dbdrop.q.out 803e35e40681df017a2f4e6d017b72749417d461 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestLocationQueries.java 31195c4523a35caf473eeecd8c8142cd1265ca18 
>   itests/util/src/main/java/org/apache/hadoop/hive/accumulo/AccumuloQTestUtil.java 956478d778be03070b242ae45d340b5a99ac9316 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCoreBlobstoreCliDriver.java 3cf5ebb3df3455f9ebe701b79f5814e40f34ccc5 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCliDriver.java 1ead1448d1e1a981068c4bdf5484346babb72ec0 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCompareCliDriver.java 6b4c6c6a794da754bcaf8c4374fd9f85d51f318f 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java 70cbf04823bb3d7208df245074eb9b732eadd18e 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java c76a70e7dddd2c13c2b42c32526398c639f041e3 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreNegativeCliDriver.java 07ae6ac206926c450c86b9519da469fc51f1d55d 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CorePerfCliDriver.java 55e744e0f3d7b266593f4c8fb82eb8539ac0a563 
>   itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseQTestUtil.java 07df0c9d1ed7fa3bf28325a1c37acf043d8ca848 
>   itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseTestSetup.java 7b203a928110101ba0de695acacadef1f1484b44 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestArguments.java PRE-CREATION 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 5adbb63693b1f2fafa47939b2833310f5dd96bf2 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/parse/CoreParseNegative.java 8f5744d2f1c8c059b4148aafde0558d76b723b65 
> 
> 
> Diff: https://reviews.apache.org/r/68496/diff/1/
> 
> 
> Testing
> -------
> 
> fixed existing tests
> 
> 
> Thanks,
> 
> denys kuzmenko
> 
>


Re: Review Request 68496: Optimized & cleaned up HBaseQTest runner

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/68496/#review207858
-----------------------------------------------------------



Thank you Denis for the patch!
Nice work, just 1 question and several nit below.
Thanks,
Peter


itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
Line 63 (original)
<https://reviews.apache.org/r/68496/#comment291420>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
Lines 77 (patched)
<https://reviews.apache.org/r/68496/#comment291419>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
Line 82 (original), 82 (patched)
<https://reviews.apache.org/r/68496/#comment291415>

    Since we removed clearTestSideEffects here, are we sure that clearPostTestEffects does the same removal?



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
Line 111 (original)
<https://reviews.apache.org/r/68496/#comment291416>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
Line 113 (original)
<https://reviews.apache.org/r/68496/#comment291417>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java
Lines 130 (patched)
<https://reviews.apache.org/r/68496/#comment291418>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java
Line 78 (original), 77 (patched)
<https://reviews.apache.org/r/68496/#comment291421>

    Same as above. Does clearPostEffects is enough?



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java
Lines 106 (patched)
<https://reviews.apache.org/r/68496/#comment291422>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.



itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java
Line 129 (original)
<https://reviews.apache.org/r/68496/#comment291423>

    nit: Unnecessary change. If we decide for any reason to create a new patch. Please remove.


- Peter Vary


On aug. 24, 2018, 7:28 de, denys kuzmenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68496/
> -----------------------------------------------------------
> 
> (Updated aug. 24, 2018, 7:28 de)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-20394
>     https://issues.apache.org/jira/browse/HIVE-20394
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> 1. Set proper cluster destroy order
> 2. Propagated proper HBaseTestContext
> 3. Ported downstream fixes (CDH-63695)
> 4. General clean up
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/test/queries/negative/cascade_dbdrop.q 48be8cd07018d005a28ad8070e13c51a46cf6d06 
>   hbase-handler/src/test/queries/positive/hbase_handler_snapshot.q e4290717a85a462a45bc14e6295fa7eccad8a51d 
>   hbase-handler/src/test/results/negative/cascade_dbdrop.q.out 803e35e40681df017a2f4e6d017b72749417d461 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestLocationQueries.java 31195c4523a35caf473eeecd8c8142cd1265ca18 
>   itests/util/src/main/java/org/apache/hadoop/hive/accumulo/AccumuloQTestUtil.java 956478d778be03070b242ae45d340b5a99ac9316 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCoreBlobstoreCliDriver.java 3cf5ebb3df3455f9ebe701b79f5814e40f34ccc5 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCliDriver.java 1ead1448d1e1a981068c4bdf5484346babb72ec0 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCompareCliDriver.java 6b4c6c6a794da754bcaf8c4374fd9f85d51f318f 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java 70cbf04823bb3d7208df245074eb9b732eadd18e 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java c76a70e7dddd2c13c2b42c32526398c639f041e3 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreNegativeCliDriver.java 07ae6ac206926c450c86b9519da469fc51f1d55d 
>   itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CorePerfCliDriver.java 55e744e0f3d7b266593f4c8fb82eb8539ac0a563 
>   itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseQTestUtil.java 07df0c9d1ed7fa3bf28325a1c37acf043d8ca848 
>   itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseTestSetup.java 7b203a928110101ba0de695acacadef1f1484b44 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestArguments.java PRE-CREATION 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 5adbb63693b1f2fafa47939b2833310f5dd96bf2 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/parse/CoreParseNegative.java 8f5744d2f1c8c059b4148aafde0558d76b723b65 
> 
> 
> Diff: https://reviews.apache.org/r/68496/diff/1/
> 
> 
> Testing
> -------
> 
> fixed existing tests
> 
> 
> Thanks,
> 
> denys kuzmenko
> 
>