> On May 10, 2018, noon, Sahil Takiar wrote: > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > > Lines 674 (patched) > > <https://reviews.apache.org/r/67023/diff/1/?file=2018287#file2018287line674> > > > > is this necessary since you set the cluster type to mr above? > > Andrew Sherman wrote: > Ha good question. Yes it is necessary as setClusterType() does not always > set the cluster type :-( - it allows the cluster type to overridden with > -Dclustermode=xxx > > Sahil Takiar wrote: > interesting, should we handle other cluster types like Spark or MR too?
looks like it does already > On May 10, 2018, noon, Sahil Takiar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java > > Lines 545 (patched) > > <https://reviews.apache.org/r/67023/diff/1/?file=2018292#file2018292line545> > > > > whats the cache for? can q-tests even specify custom URIs? whats the > > use case for support multiple fs URIs? > > Andrew Sherman wrote: > OK I admit I copied this code from the way hdfsEncryptionShims works > without fully understanding it. > > Sahil Takiar wrote: > can we delete it then? i didn't realize this would require modifying code > outside of itests, so I think we should make any changes to core Hive as > minimal as possible deleted > On May 10, 2018, noon, Sahil Takiar wrote: > > shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java > > Lines 699 (patched) > > <https://reviews.apache.org/r/67023/diff/1/?file=2018300#file2018300line699> > > > > whats the use case for this dummy class? so we can run ec tests on > > hadoop versions that don't support ec? wouldn't be just disable the > > clidriver entirely for versions that don't support ec? > > Andrew Sherman wrote: > I'm imagining wanting to run a test on bith EC and non-EC > > Sahil Takiar wrote: > i thought the `NoopHdfsErasureCodingShim` was used when "the hadoop > version does not support hdfs Erasure Coding". u can still run on EC and > non-EC folders without this, right? Yes that is the use of NoopHdfsErasureCodingShim. I think you always need 2 implementations of the interface > On May 10, 2018, noon, Sahil Takiar wrote: > > shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java > > Lines 741 (patched) > > <https://reviews.apache.org/r/67023/diff/1/?file=2018300#file2018300line741> > > > > since we have a cache anyway, would it make more sense to just remove > > this and make it a loading cache? > > Andrew Sherman wrote: > I don't know, maybe. There is some advantage in keeping the same > structure as the HdfsEncryptionShim. > > Sahil Takiar wrote: > see comment above about removing the cache OK I have removed the cache and it does make the code simpler - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67023/#review202831 ----------------------------------------------------------- On May 11, 2018, 11:38 p.m., Andrew Sherman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67023/ > ----------------------------------------------------------- > > (Updated May 11, 2018, 11:38 p.m.) > > > Review request for hive and Sahil Takiar. > > > Repository: hive-git > > > Description > ------- > > TestErasureCodingHDFSCliDriver uses a test-only CommandProcessor > "ErasureProcessor" > which allows .q files to contain Erasure Coding commands similar to those > provided > by the hdfs ec command > https://hadoop.apache.org/docs/current/hadoop-project-dist/hadoop-hdfs/HDFSErasureCoding.html. > The Erasure Coding functionality is exposed through a new shim > "HdfsFileErasureCodingPolicy". > At this stage there are two .q files: > erasure_commnds.q (a simple test to show ERASURE commands can run on local fs > via > TestCliDriver or on hdfs via TestErasureCodingHDFSCliDriver), and > erasure_simple.q (which does some trivial queries to demonstrate basic > functionality). > More tests will come in future commits. > > > Diffs > ----- > > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestErasureCodingHDFSCliDriver.java > PRE-CREATION > itests/src/test/resources/testconfiguration.properties > cf6d19a5937c3f4a82e4ffe09201af8a79da2e3d > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > 6628336807b06cab49063673be0d8e9c5b5a7101 > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java > 750fc69c5f210ca8f7bfe81b82ee9e3001fc07ba > > ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java > 3d47991b603c94c8da2106e67192c8513ef783a7 > ql/src/java/org/apache/hadoop/hive/ql/processors/ErasureProcessor.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java > 56c7516ecfaf2421b0f3d3a188d05f38715b25b2 > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java > 9f65a771f95a7c0bd3fdb4e56e47c0fc70235850 > > ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java > de43c2866f64e2ed5c74eab450de28f1a79248dc > ql/src/test/queries/clientpositive/erasure_commands.q PRE-CREATION > ql/src/test/queries/clientpositive/erasure_simple.q PRE-CREATION > ql/src/test/results/clientpositive/erasure_commands.q.out PRE-CREATION > ql/src/test/results/clientpositive/erasurecoding/erasure_commands.q.out > PRE-CREATION > ql/src/test/results/clientpositive/erasurecoding/erasure_simple.q.out > PRE-CREATION > shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java > ec06a88dc21346473bec6589c703167d50e3b367 > shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java > b89081761780bf1f305d0196bb94bb0b54f7184f > testutils/ptest2/conf/deployed/master-mr2.properties > 7edc307f85744d60d322ad8087164625677fc230 > > > Diff: https://reviews.apache.org/r/67023/diff/2/ > > > Testing > ------- > > > Thanks, > > Andrew Sherman > >
