You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/03/20 08:03:05 UTC
[GitHub] [incubator-hudi] hddong opened a new pull request #1424:
[HUDI-697]Add unit test for ArchivedCommitsCommand
hddong opened a new pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424
## *Tips*
- *Thank you very much for contributing to Apache Hudi.*
- *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
## What is the purpose of the pull request
*Add unit test for ArchivedCommitsCommand in hudi-cli module*
## Brief change log
- *Add unit test for ArchivedCommitsCommand*
## Verify this pull request
- *Add unit test for ArchivedCommitsCommand*
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] codecov-io edited a comment on issue #1424:
[HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-601586198
# [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=h1) Report
> Merging [#1424](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/14e0c95206f6d7c1806555490bcbce8785ffea5a&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1424 +/- ##
============================================
+ Coverage 67.58% 67.60% +0.01%
Complexity 255 255
============================================
Files 340 340
Lines 16499 16514 +15
Branches 1687 1689 +2
============================================
+ Hits 11151 11164 +13
- Misses 4611 4612 +1
- Partials 737 738 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../hudi/execution/MergeOnReadLazyInsertIterable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvZXhlY3V0aW9uL01lcmdlT25SZWFkTGF6eUluc2VydEl0ZXJhYmxlLmphdmE=) | `64.70% <0.00%> (-1.97%)` | `0.00% <0.00%> (ø%)` | |
| [...e/hudi/common/table/log/HoodieLogFormatWriter.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9Ib29kaWVMb2dGb3JtYXRXcml0ZXIuamF2YQ==) | `75.00% <0.00%> (-1.93%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieCopyOnWriteTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllQ29weU9uV3JpdGVUYWJsZS5qYXZh) | `90.00% <0.00%> (-0.77%)` | `0.00% <0.00%> (ø%)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `72.27% <0.00%> (-0.14%)` | `38.00% <0.00%> (ø%)` | |
| [...in/java/org/apache/hudi/table/WorkloadProfile.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvV29ya2xvYWRQcm9maWxlLmphdmE=) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieMergeOnReadTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllTWVyZ2VPblJlYWRUYWJsZS5qYXZh) | `85.80% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...table/compact/HoodieMergeOnReadTableCompactor.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvY29tcGFjdC9Ib29kaWVNZXJnZU9uUmVhZFRhYmxlQ29tcGFjdG9yLmphdmE=) | `90.10% <0.00%> (+0.10%)` | `0.00% <0.00%> (ø%)` | |
| [...i/utilities/deltastreamer/HoodieDeltaStreamer.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lci5qYXZh) | `80.20% <0.00%> (+0.40%)` | `8.00% <0.00%> (ø%)` | |
| [...in/java/org/apache/hudi/io/HoodieAppendHandle.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vSG9vZGllQXBwZW5kSGFuZGxlLmphdmE=) | `84.17% <0.00%> (+0.41%)` | `0.00% <0.00%> (ø%)` | |
| [...ain/java/org/apache/hudi/io/HoodieMergeHandle.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vSG9vZGllTWVyZ2VIYW5kbGUuamF2YQ==) | `79.31% <0.00%> (+0.43%)` | `0.00% <0.00%> (ø%)` | |
| ... and [3 more](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=footer). Last update [14e0c95...e2605ab](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396192858
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestArchivedCommitsCommand.java
##########
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.AbstractShellIntegrationTest;
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.HoodiePrintHelper;
+import org.apache.hudi.cli.TableHeader;
+import org.apache.hudi.cli.common.HoodieTestCommitMetadataGenerator;
+import org.apache.hudi.cli.common.HoodieTestCommitUtilities;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.table.HoodieCommitArchiveLog;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.shell.core.CommandResult;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test Cases for {@link ArchivedCommitsCommand}.
+ */
+public class TestArchivedCommitsCommand extends AbstractShellIntegrationTest {
+
+ private String tablePath;
+
+ @Before
+ public void init() throws IOException {
+ initDFS();
+ jsc.hadoopConfiguration().addResource(dfs.getConf());
+ HoodieCLI.conf = dfs.getConf();
+
+ // Create table and connect
+ String tableName = "test_table";
+ tablePath = basePath + File.separator + tableName;
+ new TableCommand().createTable(
+ tablePath, tableName,
+ "COPY_ON_WRITE", "", 1, "org.apache.hudi.common.model.HoodieAvroPayload");
+
+ metaClient = HoodieCLI.getTableMetaClient();
+
+ // Generate archive
+ HoodieWriteConfig cfg = HoodieWriteConfig.newBuilder().withPath(tablePath)
+ .withSchema(HoodieTestCommitMetadataGenerator.TRIP_EXAMPLE_SCHEMA).withParallelism(2, 2)
+ .withCompactionConfig(HoodieCompactionConfig.newBuilder().retainCommits(1).archiveCommitsWith(2, 3).build())
+ .forTable("test-trip-table").build();
+
+ // Create six commits
+ for (int i = 100; i < 106; i++) {
+ String timestamp = String.valueOf(i);
+ // Requested Compaction
+ HoodieTestCommitMetadataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ // Inflight Compaction
+ HoodieTestCommitMetadataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ HoodieTestCommitMetadataGenerator.createCommitFileWithMetadata(tablePath, timestamp, dfs.getConf());
+ }
+
+ metaClient = HoodieTableMetaClient.reload(metaClient);
+ // reload the timeline and get all the commits before archive
+ HoodieTimeline timeline = metaClient.getActiveTimeline().reload().getAllCommitsTimeline().filterCompletedInstants();
+ assertEquals("Loaded 6 commits and the count should match", 6, timeline.countInstants());
Review comment:
IMO, we'd better to use assertion in a method that marks with `@Test` annotation. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396204261
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
> IMO, we can just make `HoodieCommitArchiveLog#commitMetadataConverter` a **static** method and rename it to `convertCommitMetadata` then you reuse it. WDYT?
Yes, this is a better solution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396192875
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestArchivedCommitsCommand.java
##########
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.AbstractShellIntegrationTest;
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.HoodiePrintHelper;
+import org.apache.hudi.cli.TableHeader;
+import org.apache.hudi.cli.common.HoodieTestCommitMetadataGenerator;
+import org.apache.hudi.cli.common.HoodieTestCommitUtilities;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.table.HoodieCommitArchiveLog;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.shell.core.CommandResult;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test Cases for {@link ArchivedCommitsCommand}.
+ */
+public class TestArchivedCommitsCommand extends AbstractShellIntegrationTest {
+
+ private String tablePath;
+
+ @Before
+ public void init() throws IOException {
+ initDFS();
+ jsc.hadoopConfiguration().addResource(dfs.getConf());
+ HoodieCLI.conf = dfs.getConf();
+
+ // Create table and connect
+ String tableName = "test_table";
+ tablePath = basePath + File.separator + tableName;
+ new TableCommand().createTable(
+ tablePath, tableName,
+ "COPY_ON_WRITE", "", 1, "org.apache.hudi.common.model.HoodieAvroPayload");
+
+ metaClient = HoodieCLI.getTableMetaClient();
+
+ // Generate archive
+ HoodieWriteConfig cfg = HoodieWriteConfig.newBuilder().withPath(tablePath)
+ .withSchema(HoodieTestCommitMetadataGenerator.TRIP_EXAMPLE_SCHEMA).withParallelism(2, 2)
+ .withCompactionConfig(HoodieCompactionConfig.newBuilder().retainCommits(1).archiveCommitsWith(2, 3).build())
+ .forTable("test-trip-table").build();
+
+ // Create six commits
+ for (int i = 100; i < 106; i++) {
+ String timestamp = String.valueOf(i);
+ // Requested Compaction
+ HoodieTestCommitMetadataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ // Inflight Compaction
+ HoodieTestCommitMetadataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ HoodieTestCommitMetadataGenerator.createCommitFileWithMetadata(tablePath, timestamp, dfs.getConf());
+ }
+
+ metaClient = HoodieTableMetaClient.reload(metaClient);
+ // reload the timeline and get all the commits before archive
+ HoodieTimeline timeline = metaClient.getActiveTimeline().reload().getAllCommitsTimeline().filterCompletedInstants();
+ assertEquals("Loaded 6 commits and the count should match", 6, timeline.countInstants());
+
+ // archive
+ HoodieCommitArchiveLog archiveLog = new HoodieCommitArchiveLog(cfg, metaClient);
+ assertTrue(archiveLog.archiveIfRequired(jsc));
Review comment:
ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] codecov-io commented on issue #1424: [HUDI-697]Add
unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-601586198
# [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=h1) Report
> Merging [#1424](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/14e0c95206f6d7c1806555490bcbce8785ffea5a&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1424 +/- ##
============================================
- Coverage 67.58% 67.54% -0.05%
+ Complexity 255 253 -2
============================================
Files 340 340
Lines 16499 16499
Branches 1687 1687
============================================
- Hits 11151 11144 -7
- Misses 4611 4618 +7
Partials 737 737
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...g/apache/hudi/metrics/InMemoryMetricsReporter.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvbWV0cmljcy9Jbk1lbW9yeU1ldHJpY3NSZXBvcnRlci5qYXZh) | `40.00% <0.00%> (-60.00%)` | `0.00% <0.00%> (ø%)` | |
| [...src/main/java/org/apache/hudi/metrics/Metrics.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvbWV0cmljcy9NZXRyaWNzLmphdmE=) | `56.75% <0.00%> (-13.52%)` | `0.00% <0.00%> (ø%)` | |
| [...che/hudi/common/util/BufferedRandomAccessFile.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvQnVmZmVyZWRSYW5kb21BY2Nlc3NGaWxlLmphdmE=) | `55.26% <0.00%> (+0.87%)` | `0.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=footer). Last update [14e0c95...92d1ff3](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396047382
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
> IIUC, `converter` is a noun while `convert` is a verb. A method usually means a behavior that starts with a verb actively. So considering the function of this method, wdyt about renaming to `convertCommitMetadata`? It's an open topic, you can share your thought if you have.
Actually, it is a copy of HoodieCommitArchiveLog$commitMetadataConverter [https://github.com/hddong/incubator-hudi/blob/Add-test-CommitsCommand/hudi-client/src/main/java/org/apache/hudi/table/HoodieCommitArchiveLog.java#L334]. Since HoodieCommitArchiveLog construct with `HoodieTableMetaClient` and `HoodieWriteConfig`, but test class and this mothod not need.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396047382
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
> IIUC, `converter` is a noun while `convert` is a verb. A method usually means a behavior that starts with a verb actively. So considering the function of this method, wdyt about renaming to `convertCommitMetadata`? It's an open topic, you can share your thought if you have.
Actually, it is a copy of HoodieCommitArchiveLog$commitMetadataConverter[https://github.com/hddong/incubator-hudi/blob/Add-test-CommitsCommand/hudi-client/src/main/java/org/apache/hudi/table/HoodieCommitArchiveLog.java#L334]. Since HoodieCommitArchiveLog construct with `HoodieTableMetaClient` and `HoodieWriteConfig`, but test class and this mothod not need.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396048302
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestArchivedCommitsCommand.java
##########
@@ -0,0 +1,195 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.AbstractShellIntegrationTest;
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.HoodiePrintHelper;
+import org.apache.hudi.cli.TableHeader;
+import org.apache.hudi.cli.common.HoodieTestCommandDataGenerator;
+import org.apache.hudi.cli.common.HoodieTestCommitOperate;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.table.HoodieCommitArchiveLog;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.shell.core.CommandResult;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test Cases for {@link ArchivedCommitsCommand}.
+ */
+public class TestArchivedCommitsCommand extends AbstractShellIntegrationTest {
+
+ private String tablePath;
+
+ @Before
+ public void init() throws IOException {
+ initDFS();
+ jsc.hadoopConfiguration().addResource(dfs.getConf());
+ HoodieCLI.conf = dfs.getConf();
+
+ // Create table and connect
+ String tableName = "test_table";
+ tablePath = basePath + File.separator + tableName;
+ new TableCommand().createTable(
+ tablePath, tableName,
+ "COPY_ON_WRITE", "", 1, "org.apache.hudi.common.model.HoodieAvroPayload");
+
+ metaClient = HoodieCLI.getTableMetaClient();
+
+ // Generate archive
+ HoodieWriteConfig cfg = HoodieWriteConfig.newBuilder().withPath(tablePath)
+ .withSchema(HoodieTestCommandDataGenerator.TRIP_EXAMPLE_SCHEMA).withParallelism(2, 2)
+ .withCompactionConfig(HoodieCompactionConfig.newBuilder().retainCommits(1).archiveCommitsWith(2, 3).build())
+ .forTable("test-trip-table").build();
+
+ // Create six commits
+ for (int i = 100; i < 106; i++) {
+ String timestamp = String.valueOf(i);
+ // Requested Compaction
+ HoodieTestCommandDataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ // Inflight Compaction
+ HoodieTestCommandDataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ HoodieTestCommandDataGenerator.createCommitFileWithMetadata(tablePath, timestamp, dfs.getConf());
+ }
+
+ metaClient = HoodieTableMetaClient.reload(metaClient);
+ // reload the timeline and get all the commits before archive
+ HoodieTimeline timeline = metaClient.getActiveTimeline().reload().getAllCommitsTimeline().filterCompletedInstants();
+ assertEquals("Loaded 6 commits and the count should match", 6, timeline.countInstants());
+
+ // archive
+ HoodieCommitArchiveLog archiveLog = new HoodieCommitArchiveLog(cfg, metaClient);
+ assertTrue(archiveLog.archiveIfRequired(jsc));
+ }
+
+ @After
+ public void clean() throws IOException {
+ cleanupDFS();
+ }
+
+ /**
+ * Test for command: show archived commit stats.
+ */
+ @Test
+ public void testShowArchivedCommits() {
+ CommandResult cr = getShell().executeCommand("show archived commit stats");
+ assertTrue(cr.isSuccess());
+
+ TableHeader header = new TableHeader().addTableHeaderField("action").addTableHeaderField("instant")
+ .addTableHeaderField("partition").addTableHeaderField("file_id").addTableHeaderField("prev_instant")
+ .addTableHeaderField("num_writes").addTableHeaderField("num_inserts").addTableHeaderField("num_deletes")
+ .addTableHeaderField("num_update_writes").addTableHeaderField("total_log_files")
+ .addTableHeaderField("total_log_blocks").addTableHeaderField("total_corrupt_log_blocks")
+ .addTableHeaderField("total_rollback_blocks").addTableHeaderField("total_log_records")
+ .addTableHeaderField("total_updated_records_compacted").addTableHeaderField("total_write_bytes")
+ .addTableHeaderField("total_write_errors");
+
+ // Generate expected data
+ final List<Comparable[]> rows = new ArrayList<>();
+ for (int i = 100; i < 104; i++) {
+ String instant = String.valueOf(i);
+ for (int j = 0; j < 3; j++) {
+ Comparable[] defaultComp = new Comparable[]{"commit", instant,
+ HoodieTestCommandDataGenerator.DEFAULT_SECOND_PARTITION_PATH,
+ HoodieTestCommandDataGenerator.DEFAULT_FILEID,
+ HoodieTestCommandDataGenerator.DEFAULT_PRE_COMMIT,
+ HoodieTestCommandDataGenerator.DEFAULT_NUM_WRITES,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_NUM_UPDATE_WRITES,
+ HoodieTestCommandDataGenerator.DEFAULT_NULL_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_TOTAL_LOG_BLOCKS,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_TOTAL_LOG_RECORDS,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_TOTAL_WRITE_BYTES,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE};
+ rows.add(defaultComp.clone());
+ defaultComp[2] = HoodieTestCommandDataGenerator.DEFAULT_FIRST_PARTITION_PATH;
+ rows.add(defaultComp);
+ }
+ }
+
+ String expectedResult = HoodiePrintHelper.print(
+ header, new HashMap<>(), "", false, -1, false, rows);
+ assertEquals(expectedResult, cr.getResult().toString());
+ }
+
+ /**
+ * Test for command: show archived commits.
+ */
+ @Test
+ public void testShowCommits() throws IOException {
+ CommandResult cr = getShell().executeCommand("show archived commits");
+ assertTrue(cr.isSuccess());
+ System.out.println(cr.getResult().toString());
Review comment:
> Why we should use `System.out.println`? Do we need to get information from STDOUT?
Removed it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] codecov-io edited a comment on issue #1424:
[HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-601586198
# [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=h1) Report
> Merging [#1424](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/14e0c95206f6d7c1806555490bcbce8785ffea5a&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1424 +/- ##
============================================
+ Coverage 67.58% 67.64% +0.05%
+ Complexity 255 254 -1
============================================
Files 340 340
Lines 16499 16504 +5
Branches 1687 1689 +2
============================================
+ Hits 11151 11164 +13
+ Misses 4611 4601 -10
- Partials 737 739 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../org/apache/hudi/table/HoodieCommitArchiveLog.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllQ29tbWl0QXJjaGl2ZUxvZy5qYXZh) | `75.00% <50.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [.../hudi/execution/MergeOnReadLazyInsertIterable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvZXhlY3V0aW9uL01lcmdlT25SZWFkTGF6eUluc2VydEl0ZXJhYmxlLmphdmE=) | `64.70% <0.00%> (-1.97%)` | `0.00% <0.00%> (ø%)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.54% <0.00%> (-1.21%)` | `12.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieCopyOnWriteTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllQ29weU9uV3JpdGVUYWJsZS5qYXZh) | `90.00% <0.00%> (-0.77%)` | `0.00% <0.00%> (ø%)` | |
| [...java/org/apache/hudi/utilities/sources/Source.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvU291cmNlLmphdmE=) | `87.50% <0.00%> (-0.74%)` | `5.00% <0.00%> (-1.00%)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `72.27% <0.00%> (-0.14%)` | `38.00% <0.00%> (ø%)` | |
| [...in/java/org/apache/hudi/table/WorkloadProfile.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvV29ya2xvYWRQcm9maWxlLmphdmE=) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieMergeOnReadTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllTWVyZ2VPblJlYWRUYWJsZS5qYXZh) | `85.80% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...table/compact/HoodieMergeOnReadTableCompactor.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvY29tcGFjdC9Ib29kaWVNZXJnZU9uUmVhZFRhYmxlQ29tcGFjdG9yLmphdmE=) | `90.10% <0.00%> (+0.10%)` | `0.00% <0.00%> (ø%)` | |
| [...i/utilities/deltastreamer/HoodieDeltaStreamer.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lci5qYXZh) | `80.20% <0.00%> (+0.40%)` | `8.00% <0.00%> (ø%)` | |
| ... and [6 more](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=footer). Last update [14e0c95...f23724f](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r395700713
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommandDataGenerator.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieTestUtils;
+import org.apache.hudi.common.model.HoodieWriteStat;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.util.FSUtils;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Class to be used in tests to keep generating test inserts and updates against a corpus.
+ */
+public class HoodieTestCommandDataGenerator extends HoodieTestDataGenerator {
+
+ // default commit metadata value
+ public static final String DEFAULT_PATH = "path";
+ public static final String DEFAULT_FILEID = "fileId";
+ public static final int DEFAULT_TOTAL_WRITE_BYTES = 50;
+ public static final String DEFAULT_PRE_COMMIT = "commit-1";
+ public static final int DEFAULT_NUM_WRITES = 10;
+ public static final int DEFAULT_NUM_UPDATE_WRITES = 15;
+ public static final int DEFAULT_TOTAL_LOG_BLOCKS = 1;
+ public static final int DEFAULT_TOTAL_LOG_RECORDS = 10;
+ public static final int DEFAULT_OTHER_VALUE = 0;
+ public static final String DEFAULT_NULL_VALUE = "null";
+
+ /**
+ * Create a commit file with default CommitMetadata.
+ */
+ public static void createCommitFileWithMetadata(String basePath, String commitTime, Configuration configuration) {
+ Arrays.asList(HoodieTimeline.makeCommitFileName(commitTime), HoodieTimeline.makeInflightCommitFileName(commitTime),
+ HoodieTimeline.makeRequestedCommitFileName(commitTime))
+ .forEach(f -> {
+ Path commitFile = new Path(
+ basePath + "/" + HoodieTableMetaClient.METAFOLDER_NAME + "/" + f);
+ FSDataOutputStream os = null;
+ try {
+ FileSystem fs = FSUtils.getFs(basePath, configuration);
+ os = fs.create(commitFile, true);
+ // Generate commitMetadata
+ HoodieCommitMetadata commitMetadata = generateCommitMetadata(basePath);
+ // Write empty commit metadata
+ os.writeBytes(new String(commitMetadata.toJsonString().getBytes(StandardCharsets.UTF_8)));
+ } catch (IOException ioe) {
+ throw new HoodieIOException(ioe.getMessage(), ioe);
+ } finally {
+ if (null != os) {
+ try {
+ os.close();
+ } catch (IOException e) {
+ throw new HoodieIOException(e.getMessage(), e);
+ }
+ }
+ }
+ });
+ }
+
+ /**
+ * Generate commitMetadata in path.
+ */
+ public static HoodieCommitMetadata generateCommitMetadata(String basePath) throws IOException {
+ String file1P0C0 =
+ HoodieTestUtils.createNewDataFile(basePath, DEFAULT_FIRST_PARTITION_PATH, "000");
+ String file1P1C0 =
+ HoodieTestUtils.createNewDataFile(basePath, DEFAULT_SECOND_PARTITION_PATH, "000");
+ return generateCommitMetadata(new ImmutableMap.Builder()
+ .put(DEFAULT_FIRST_PARTITION_PATH, new ImmutableList.Builder<>().add(file1P0C0).build())
+ .put(DEFAULT_SECOND_PARTITION_PATH, new ImmutableList.Builder<>().add(file1P1C0).build())
+ .build());
+ }
+
+
+
+ /**
+ * Method to generate commit metadata.
+ */
+ public static HoodieCommitMetadata generateCommitMetadata(Map<String, List<String>> partitionToFilePaths) {
Review comment:
can we modify the access modifier to `private`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396192544
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommandDataGenerator.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieTestUtils;
+import org.apache.hudi.common.model.HoodieWriteStat;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.util.FSUtils;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Class to be used in tests to keep generating test inserts and updates against a corpus.
+ */
+public class HoodieTestCommandDataGenerator extends HoodieTestDataGenerator {
Review comment:
OK, just saw you reused the method from `HoodieTestDataGenerator `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396048962
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
IMO, we can move [HoodieCommitArchiveLog$commitMetadataConverter](https://github.com/hddong/incubator-hudi/blob/Add-test-CommitsCommand/hudi-client/src/main/java/org/apache/hudi/table/HoodieCommitArchiveLog.java#L334) to `ClientUtils`, as it is a pubilce mothod now, wdyt.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on issue #1424: [HUDI-697]Add
unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-601607599
@yanghua @vinothchandar please have a review when free.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396048962
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
IMO, we can move [HoodieCommitArchiveLog$commitMetadataConverter] to `ClientUtils`, wdyt.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396048282
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommandDataGenerator.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieTestUtils;
+import org.apache.hudi.common.model.HoodieWriteStat;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.util.FSUtils;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Class to be used in tests to keep generating test inserts and updates against a corpus.
+ */
+public class HoodieTestCommandDataGenerator extends HoodieTestDataGenerator {
Review comment:
> Why we need to extends `HoodieTestDataGenerator `? We only reused some static field here, right? And wdyt about renaming to `HoodieTestCommitMetadataGenerator` based on the implementation of this class.
IMO, this class is extension of `HoodieTestDataGenerator `, add some field and method for hudi-cli base on `HoodieTestDataGenerator`. Name you sugguested is better here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396047382
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
> IIUC, `converter` is a noun while `convert` is a verb. A method usually means a behavior that starts with a verb actively. So considering the function of this method, wdyt about renaming to `convertCommitMetadata`? It's an open topic, you can share your thought if you have.
Actually, it is a copy of [HoodieCommitArchiveLog$commitMetadataConverter](https://github.com/hddong/incubator-hudi/blob/Add-test-CommitsCommand/hudi-client/src/main/java/org/apache/hudi/table/HoodieCommitArchiveLog.java#L334). Since HoodieCommitArchiveLog construct with `HoodieTableMetaClient` and `HoodieWriteConfig`, but test class and this mothod not need. Waht you said is right, but may be keep identical is better here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] codecov-io edited a comment on issue #1424:
[HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-601586198
# [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=h1) Report
> Merging [#1424](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/14e0c95206f6d7c1806555490bcbce8785ffea5a&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1424 +/- ##
============================================
+ Coverage 67.58% 67.60% +0.01%
Complexity 255 255
============================================
Files 340 340
Lines 16499 16514 +15
Branches 1687 1689 +2
============================================
+ Hits 11151 11164 +13
- Misses 4611 4612 +1
- Partials 737 738 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../hudi/execution/MergeOnReadLazyInsertIterable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvZXhlY3V0aW9uL01lcmdlT25SZWFkTGF6eUluc2VydEl0ZXJhYmxlLmphdmE=) | `64.70% <0.00%> (-1.97%)` | `0.00% <0.00%> (ø%)` | |
| [...e/hudi/common/table/log/HoodieLogFormatWriter.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9Ib29kaWVMb2dGb3JtYXRXcml0ZXIuamF2YQ==) | `75.00% <0.00%> (-1.93%)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieCopyOnWriteTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllQ29weU9uV3JpdGVUYWJsZS5qYXZh) | `90.00% <0.00%> (-0.77%)` | `0.00% <0.00%> (ø%)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `72.27% <0.00%> (-0.14%)` | `38.00% <0.00%> (ø%)` | |
| [...in/java/org/apache/hudi/table/WorkloadProfile.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvV29ya2xvYWRQcm9maWxlLmphdmE=) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieMergeOnReadTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllTWVyZ2VPblJlYWRUYWJsZS5qYXZh) | `85.80% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...table/compact/HoodieMergeOnReadTableCompactor.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvY29tcGFjdC9Ib29kaWVNZXJnZU9uUmVhZFRhYmxlQ29tcGFjdG9yLmphdmE=) | `90.10% <0.00%> (+0.10%)` | `0.00% <0.00%> (ø%)` | |
| [...i/utilities/deltastreamer/HoodieDeltaStreamer.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lci5qYXZh) | `80.20% <0.00%> (+0.40%)` | `8.00% <0.00%> (ø%)` | |
| [...in/java/org/apache/hudi/io/HoodieAppendHandle.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vSG9vZGllQXBwZW5kSGFuZGxlLmphdmE=) | `84.17% <0.00%> (+0.41%)` | `0.00% <0.00%> (ø%)` | |
| [...ain/java/org/apache/hudi/io/HoodieMergeHandle.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vSG9vZGllTWVyZ2VIYW5kbGUuamF2YQ==) | `79.31% <0.00%> (+0.43%)` | `0.00% <0.00%> (ø%)` | |
| ... and [3 more](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=footer). Last update [14e0c95...e2605ab](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396047382
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
> IIUC, `converter` is a noun while `convert` is a verb. A method usually means a behavior that starts with a verb actively. So considering the function of this method, wdyt about renaming to `convertCommitMetadata`? It's an open topic, you can share your thought if you have.
Actually, it is a copy of [HoodieCommitArchiveLog$commitMetadataConverter](https://github.com/hddong/incubator-hudi/blob/Add-test-CommitsCommand/hudi-client/src/main/java/org/apache/hudi/table/HoodieCommitArchiveLog.java#L334). Since HoodieCommitArchiveLog construct with `HoodieTableMetaClient` and `HoodieWriteConfig`, but test class and this mothod not need.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r395688239
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
Review comment:
Will we reuse this class in the future for other test cases? If no, can we move these utility methods into `TestArchivedCommitsCommand`. If yes, what do you think about renaming to `HoodieTestCommitUtilities`. It seems `utility` is more clear than `operate` here. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r395699029
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommandDataGenerator.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieTestUtils;
+import org.apache.hudi.common.model.HoodieWriteStat;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.util.FSUtils;
+import org.apache.hudi.exception.HoodieIOException;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Class to be used in tests to keep generating test inserts and updates against a corpus.
+ */
+public class HoodieTestCommandDataGenerator extends HoodieTestDataGenerator {
Review comment:
Why we need to extends `HoodieTestDataGenerator `? We only reused some static field here, right? And wdyt about renaming to `HoodieTestCommitMetadataGenerator` based on the implementation of this class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua merged pull request #1424: [HUDI-697]Add
unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r395692451
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
IIUC, `converter` is a noun while `convert` is a verb. A method usually means a behavior that starts with a verb actively. So considering the function of this method, wdyt about renaming to `convertCommitMetadata`? It's an open topic, you can share your thought if you have.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396191728
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
+ HoodieCommitMetadata hoodieCommitMetadata) {
+ return orderCommitMetadata(commitMetadataConverter(hoodieCommitMetadata));
+ }
+
+ /**
+ * Converter HoodieCommitMetadata to avro format.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverter(
Review comment:
IMO, we can just make `HoodieCommitArchiveLog#commitMetadataConverter` a **static** method and rename it to `convertCommitMetadata` then you reuse it. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396203068
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestArchivedCommitsCommand.java
##########
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.AbstractShellIntegrationTest;
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.HoodiePrintHelper;
+import org.apache.hudi.cli.TableHeader;
+import org.apache.hudi.cli.common.HoodieTestCommitMetadataGenerator;
+import org.apache.hudi.cli.common.HoodieTestCommitUtilities;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.table.HoodieCommitArchiveLog;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.shell.core.CommandResult;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test Cases for {@link ArchivedCommitsCommand}.
+ */
+public class TestArchivedCommitsCommand extends AbstractShellIntegrationTest {
+
+ private String tablePath;
+
+ @Before
+ public void init() throws IOException {
+ initDFS();
+ jsc.hadoopConfiguration().addResource(dfs.getConf());
+ HoodieCLI.conf = dfs.getConf();
+
+ // Create table and connect
+ String tableName = "test_table";
+ tablePath = basePath + File.separator + tableName;
+ new TableCommand().createTable(
+ tablePath, tableName,
+ "COPY_ON_WRITE", "", 1, "org.apache.hudi.common.model.HoodieAvroPayload");
+
+ metaClient = HoodieCLI.getTableMetaClient();
+
+ // Generate archive
+ HoodieWriteConfig cfg = HoodieWriteConfig.newBuilder().withPath(tablePath)
+ .withSchema(HoodieTestCommitMetadataGenerator.TRIP_EXAMPLE_SCHEMA).withParallelism(2, 2)
+ .withCompactionConfig(HoodieCompactionConfig.newBuilder().retainCommits(1).archiveCommitsWith(2, 3).build())
+ .forTable("test-trip-table").build();
+
+ // Create six commits
+ for (int i = 100; i < 106; i++) {
+ String timestamp = String.valueOf(i);
+ // Requested Compaction
+ HoodieTestCommitMetadataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ // Inflight Compaction
+ HoodieTestCommitMetadataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ HoodieTestCommitMetadataGenerator.createCommitFileWithMetadata(tablePath, timestamp, dfs.getConf());
+ }
+
+ metaClient = HoodieTableMetaClient.reload(metaClient);
+ // reload the timeline and get all the commits before archive
+ HoodieTimeline timeline = metaClient.getActiveTimeline().reload().getAllCommitsTimeline().filterCompletedInstants();
+ assertEquals("Loaded 6 commits and the count should match", 6, timeline.countInstants());
Review comment:
> IMO, we'd better to use assertion in a method that marks with `@Test` annotation. WDYT?
Yes, I will remove assertion here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r395692986
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
+
+ /**
+ * Converter HoodieCommitMetadata to avro format and ordered by partition.
+ */
+ public static org.apache.hudi.avro.model.HoodieCommitMetadata commitMetadataConverterOrdered(
Review comment:
same thought about `converter` see other relevant comments in this class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] codecov-io edited a comment on issue #1424:
[HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-601586198
# [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=h1) Report
> Merging [#1424](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/14e0c95206f6d7c1806555490bcbce8785ffea5a&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1424 +/- ##
============================================
+ Coverage 67.58% 67.64% +0.05%
+ Complexity 255 254 -1
============================================
Files 340 340
Lines 16499 16504 +5
Branches 1687 1689 +2
============================================
+ Hits 11151 11164 +13
+ Misses 4611 4601 -10
- Partials 737 739 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../org/apache/hudi/table/HoodieCommitArchiveLog.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllQ29tbWl0QXJjaGl2ZUxvZy5qYXZh) | `75.00% <50.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [.../hudi/execution/MergeOnReadLazyInsertIterable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvZXhlY3V0aW9uL01lcmdlT25SZWFkTGF6eUluc2VydEl0ZXJhYmxlLmphdmE=) | `64.70% <0.00%> (-1.97%)` | `0.00% <0.00%> (ø%)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.54% <0.00%> (-1.21%)` | `12.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieCopyOnWriteTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllQ29weU9uV3JpdGVUYWJsZS5qYXZh) | `90.00% <0.00%> (-0.77%)` | `0.00% <0.00%> (ø%)` | |
| [...java/org/apache/hudi/utilities/sources/Source.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvU291cmNlLmphdmE=) | `87.50% <0.00%> (-0.74%)` | `5.00% <0.00%> (-1.00%)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `72.27% <0.00%> (-0.14%)` | `38.00% <0.00%> (ø%)` | |
| [...in/java/org/apache/hudi/table/WorkloadProfile.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvV29ya2xvYWRQcm9maWxlLmphdmE=) | `100.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [.../org/apache/hudi/table/HoodieMergeOnReadTable.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvSG9vZGllTWVyZ2VPblJlYWRUYWJsZS5qYXZh) | `85.80% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...table/compact/HoodieMergeOnReadTableCompactor.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdGFibGUvY29tcGFjdC9Ib29kaWVNZXJnZU9uUmVhZFRhYmxlQ29tcGFjdG9yLmphdmE=) | `90.10% <0.00%> (+0.10%)` | `0.00% <0.00%> (ø%)` | |
| [...i/utilities/deltastreamer/HoodieDeltaStreamer.java](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lci5qYXZh) | `80.20% <0.00%> (+0.40%)` | `8.00% <0.00%> (ø%)` | |
| ... and [6 more](https://codecov.io/gh/apache/incubator-hudi/pull/1424/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=footer). Last update [14e0c95...f23724f](https://codecov.io/gh/apache/incubator-hudi/pull/1424?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] yanghua commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r395704673
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestArchivedCommitsCommand.java
##########
@@ -0,0 +1,195 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.AbstractShellIntegrationTest;
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.HoodiePrintHelper;
+import org.apache.hudi.cli.TableHeader;
+import org.apache.hudi.cli.common.HoodieTestCommandDataGenerator;
+import org.apache.hudi.cli.common.HoodieTestCommitOperate;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.table.HoodieCommitArchiveLog;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.shell.core.CommandResult;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test Cases for {@link ArchivedCommitsCommand}.
+ */
+public class TestArchivedCommitsCommand extends AbstractShellIntegrationTest {
+
+ private String tablePath;
+
+ @Before
+ public void init() throws IOException {
+ initDFS();
+ jsc.hadoopConfiguration().addResource(dfs.getConf());
+ HoodieCLI.conf = dfs.getConf();
+
+ // Create table and connect
+ String tableName = "test_table";
+ tablePath = basePath + File.separator + tableName;
+ new TableCommand().createTable(
+ tablePath, tableName,
+ "COPY_ON_WRITE", "", 1, "org.apache.hudi.common.model.HoodieAvroPayload");
+
+ metaClient = HoodieCLI.getTableMetaClient();
+
+ // Generate archive
+ HoodieWriteConfig cfg = HoodieWriteConfig.newBuilder().withPath(tablePath)
+ .withSchema(HoodieTestCommandDataGenerator.TRIP_EXAMPLE_SCHEMA).withParallelism(2, 2)
+ .withCompactionConfig(HoodieCompactionConfig.newBuilder().retainCommits(1).archiveCommitsWith(2, 3).build())
+ .forTable("test-trip-table").build();
+
+ // Create six commits
+ for (int i = 100; i < 106; i++) {
+ String timestamp = String.valueOf(i);
+ // Requested Compaction
+ HoodieTestCommandDataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.REQUESTED, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ // Inflight Compaction
+ HoodieTestCommandDataGenerator.createCompactionAuxiliaryMetadata(tablePath,
+ new HoodieInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMPACTION_ACTION, timestamp), dfs.getConf());
+ HoodieTestCommandDataGenerator.createCommitFileWithMetadata(tablePath, timestamp, dfs.getConf());
+ }
+
+ metaClient = HoodieTableMetaClient.reload(metaClient);
+ // reload the timeline and get all the commits before archive
+ HoodieTimeline timeline = metaClient.getActiveTimeline().reload().getAllCommitsTimeline().filterCompletedInstants();
+ assertEquals("Loaded 6 commits and the count should match", 6, timeline.countInstants());
+
+ // archive
+ HoodieCommitArchiveLog archiveLog = new HoodieCommitArchiveLog(cfg, metaClient);
+ assertTrue(archiveLog.archiveIfRequired(jsc));
+ }
+
+ @After
+ public void clean() throws IOException {
+ cleanupDFS();
+ }
+
+ /**
+ * Test for command: show archived commit stats.
+ */
+ @Test
+ public void testShowArchivedCommits() {
+ CommandResult cr = getShell().executeCommand("show archived commit stats");
+ assertTrue(cr.isSuccess());
+
+ TableHeader header = new TableHeader().addTableHeaderField("action").addTableHeaderField("instant")
+ .addTableHeaderField("partition").addTableHeaderField("file_id").addTableHeaderField("prev_instant")
+ .addTableHeaderField("num_writes").addTableHeaderField("num_inserts").addTableHeaderField("num_deletes")
+ .addTableHeaderField("num_update_writes").addTableHeaderField("total_log_files")
+ .addTableHeaderField("total_log_blocks").addTableHeaderField("total_corrupt_log_blocks")
+ .addTableHeaderField("total_rollback_blocks").addTableHeaderField("total_log_records")
+ .addTableHeaderField("total_updated_records_compacted").addTableHeaderField("total_write_bytes")
+ .addTableHeaderField("total_write_errors");
+
+ // Generate expected data
+ final List<Comparable[]> rows = new ArrayList<>();
+ for (int i = 100; i < 104; i++) {
+ String instant = String.valueOf(i);
+ for (int j = 0; j < 3; j++) {
+ Comparable[] defaultComp = new Comparable[]{"commit", instant,
+ HoodieTestCommandDataGenerator.DEFAULT_SECOND_PARTITION_PATH,
+ HoodieTestCommandDataGenerator.DEFAULT_FILEID,
+ HoodieTestCommandDataGenerator.DEFAULT_PRE_COMMIT,
+ HoodieTestCommandDataGenerator.DEFAULT_NUM_WRITES,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_NUM_UPDATE_WRITES,
+ HoodieTestCommandDataGenerator.DEFAULT_NULL_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_TOTAL_LOG_BLOCKS,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_TOTAL_LOG_RECORDS,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE,
+ HoodieTestCommandDataGenerator.DEFAULT_TOTAL_WRITE_BYTES,
+ HoodieTestCommandDataGenerator.DEFAULT_OTHER_VALUE};
+ rows.add(defaultComp.clone());
+ defaultComp[2] = HoodieTestCommandDataGenerator.DEFAULT_FIRST_PARTITION_PATH;
+ rows.add(defaultComp);
+ }
+ }
+
+ String expectedResult = HoodiePrintHelper.print(
+ header, new HashMap<>(), "", false, -1, false, rows);
+ assertEquals(expectedResult, cr.getResult().toString());
+ }
+
+ /**
+ * Test for command: show archived commits.
+ */
+ @Test
+ public void testShowCommits() throws IOException {
+ CommandResult cr = getShell().executeCommand("show archived commits");
+ assertTrue(cr.isSuccess());
+ System.out.println(cr.getResult().toString());
Review comment:
Why we should use `System.out.println`? Do we need to get information from STDOUT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on issue #1424: [HUDI-697]Add
unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on issue #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#issuecomment-602397033
@yanghua tanks for your review and suggestion. Addressed all of them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] hddong commented on a change in pull request
#1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1424: [HUDI-697]Add unit test for ArchivedCommitsCommand
URL: https://github.com/apache/incubator-hudi/pull/1424#discussion_r396046914
##########
File path: hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitOperate.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.common;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hudi.avro.model.HoodieWriteStat;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieRollingStatMetadata;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Utility methods to commit instant for test.
+ */
+public class HoodieTestCommitOperate {
Review comment:
> Will we reuse this class in the future for other test cases? If no, can we move these utility methods into `TestArchivedCommitsCommand`. If yes, what do you think about renaming to `HoodieTestCommitUtilities`. It seems `utility` is more clear than `operate` here. WDYT?
Yes, IMO, `CommitsCommand` may use this later. `utility` is better here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services