[HUDI-4209] Avoid using HDFS in HoodieClientTestHarness #7305
Conversation
xushiyan
added
the
priority:major
| @@ -50,16 +48,7 @@ | |||
| @SuppressWarnings("unchecked") | |||
| public class TestHoodieReadClient extends HoodieClientTestBase { | |||
|
|
|||
| @Override | |||
| protected void initPath() { | |||
There was a problem hiding this comment.
prefer to use the one in super class so remove this method in subclass
...client/hudi-spark-client/src/test/java/org/apache/hudi/client/TestUpdateSchemaEvolution.java
Show resolved
Hide resolved
| */ | ||
|
|
||
| /** | ||
| * @Test public void testLeaseRecovery() throws IOException, URISyntaxException, InterruptedException { Writer writer |
There was a problem hiding this comment.
as the comment mentions, this testcase is not as the name suggests, also the logic is covered in other test case, hence removing it
| /** | ||
| * A utility class about mini cluster. | ||
| */ | ||
| public class MiniClusterUtil { |
There was a problem hiding this comment.
discard this as it bundles hdfs with zookeeper, which should be discouraged for UT/FT. testcase which needs hdfs will use HdfsTestService where applicable
| @@ -102,28 +127,28 @@ public void readLocalWriteHDFS() throws Exception { | |||
| // Write generated data to hdfs (only inserts) | |||
| String readCommitTime = hdfsWriteClient.startCommit(); | |||
| LOG.info("Starting commit " + readCommitTime); | |||
| List<HoodieRecord> records = dataGen.generateInserts(readCommitTime, 100); | |||
| JavaRDD<HoodieRecord> writeRecords = jsc.parallelize(records, 1); | |||
| List<HoodieRecord> records = dataGen.generateInserts(readCommitTime, 10); | |||
There was a problem hiding this comment.
some of the test assumes that records will span across all 3 partition paths (HoodieTestDataGenerator). if we reduce the num of records from 100 to 10, chances we might see some flakiness. something to keep in mind.
There was a problem hiding this comment.
this test case does not depend on all partitions having records. so i've only changed this one
| @@ -89,7 +88,7 @@ public void tearDown() throws Exception { | |||
| } | |||
|
|
|||
| @ParameterizedTest | |||
| @ValueSource(booleans = {true}) | |||
| @ValueSource(booleans = {false, true}) | |||
There was a problem hiding this comment.
do we know why this was not having "false" before. may be we had some reason to remove it explicitly.
There was a problem hiding this comment.
false case was removed in #6313
not sure exactly why. but it's passing now.
| throw new HoodieIOException(ioe.getMessage(), ioe); | ||
| } | ||
| } | ||
| private static final int PARALLELISM = 2; |
There was a problem hiding this comment.
Was wondering if this can be paramterized as a maven option? Would be helpful when we run tests on bigger cluster.
There was a problem hiding this comment.
@codope i've filed this https://issues.apache.org/jira/browse/HUDI-5287
in principle we should make UT/FT only run against small number of records and runnable even on cheap machines. So we should be able to keep parallelism low and fixed. We only make num records and parallelism configurable when running ITs with varying number of records.
xushiyan
force-pushed
the
HUDI-4209-remove-hdfs
branch
from
69abf45 to
10fb959
Compare
Change Logs
Remove HDFS usage from
HoodieClientTestHarnessand subclasses.Impact
Improve CI test stability and reduce run time.
Risk level
None.
Documentation Update
NA
Contributor's checklist