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/09/16 04:14:51 UTC
[GitHub] [hudi] n3nash opened a new pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
n3nash opened a new pull request #2092:
URL: https://github.com/apache/hudi/pull/2092
## *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
Fix MergeOnRead DAG for docker demo.
This pull request is a trivial config change for testing without any test coverage.
## 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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r505103654
##########
File path: hudi-integ-test/src/test/resources/unit-test-cow-dag.yaml
##########
@@ -17,23 +17,53 @@ first_insert:
config:
record_size: 70000
num_insert_partitions: 1
- repeat_count: 1
+ repeat_count: 2
Review comment:
@n3nash : my bad. can you clarify what does repeat do here. will it add 2* 100 records ? If yes, total expected records should be 400 right in line 66.
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/RollbackNode.java
##########
@@ -49,6 +54,11 @@ public void execute(ExecutionContext executionContext) throws Exception {
Option<HoodieInstant> lastInstant = metaClient.getActiveTimeline().getCommitsTimeline().lastInstant();
if (lastInstant.isPresent()) {
Review comment:
my bad. I missed that we can rollback only the latest commit.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-718033465
@nsivabalan I've corrected the unit test DAGs, lets merge this PR and then we can do another full audit of the other dags.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r504819599
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/helpers/DFSTestSuitePathSelector.java
##########
@@ -62,19 +67,26 @@ public DFSTestSuitePathSelector(TypedProperties props, Configuration hadoopConf)
lastBatchId = 0;
nextBatchId = 1;
}
-
- log.info("Using DFSTestSuitePathSelector, checkpoint: " + lastCheckpointStr + " sourceLimit: " + sourceLimit
- + " lastBatchId: " + lastBatchId + " nextBatchId: " + nextBatchId);
// obtain all eligible files for the batch
List<FileStatus> eligibleFiles = new ArrayList<>();
FileStatus[] fileStatuses = fs.globStatus(
new Path(props.getString(Config.ROOT_INPUT_PATH_PROP), "*"));
+ // Say input data is as follow input/1, input/2, input/5 since 3,4 was rolled back and 5 is new generated data
+ // checkpoint from the latest commit metadata will be 2 since 3,4 has been rolled back. We need to set the
+ // next batch id correctly as 5 instead of 3
+ Optional<String> correctBatchIdDueToRollback = Arrays.stream(fileStatuses)
+ .map(f -> f.getPath().toString().split("/")[f.getPath().toString().split("/").length - 1])
+ .min((bid1, bid2) -> Integer.min(Integer.parseInt(bid1), Integer.parseInt(bid2)));
Review comment:
Had some uncomitted changes, pushed it now
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/RollbackNode.java
##########
@@ -49,6 +54,11 @@ public void execute(ExecutionContext executionContext) throws Exception {
Option<HoodieInstant> lastInstant = metaClient.getActiveTimeline().getCommitsTimeline().lastInstant();
if (lastInstant.isPresent()) {
log.info("Rolling back last instant {}", lastInstant.get());
+ log.info("Cleaning up generated data for the instant being rolled back {}", lastInstant.get());
+ ValidationUtils.checkArgument(executionContext.getWriterContext().getProps().getOrDefault(DFSPathSelector.Config.SOURCE_INPUT_SELECTOR,
+ DFSPathSelector.class.getName()).toString().equalsIgnoreCase(DFSTestSuitePathSelector.class.getName()), "Test Suite only supports DFSTestSuitePathSelector");
+ metaClient.getFs().delete(new Path(executionContext.getWriterContext().getCfg().inputBasePath,
Review comment:
Done
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-710060014
LGTM. Do fix the title and description since you have fixed the rollback as well.
Once you are done, let me know. or feel free to go ahead and merge 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
[GitHub] [hudi] codecov-io commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-716293969
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2092?src=pr&el=h1) Report
> Merging [#2092](https://codecov.io/gh/apache/hudi/pull/2092?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/49e855c3488ebe53af3a72fc3b5a22ba47a45e01?el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2092?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2092 +/- ##
=========================================
Coverage 53.68% 53.68%
Complexity 2849 2849
=========================================
Files 359 359
Lines 16565 16565
Branches 1782 1782
=========================================
Hits 8893 8893
Misses 6915 6915
Partials 757 757
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (ø)` | `193.00 <ø> (ø)` | |
| #hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| #hudicommon | `54.70% <ø> (ø)` | `1794.00 <ø> (ø)` | |
| #hudihadoopmr | `33.05% <ø> (ø)` | `181.00 <ø> (ø)` | |
| #hudispark | `65.95% <ø> (ø)` | `304.00 <ø> (ø)` | |
| #huditimelineservice | `62.29% <ø> (ø)` | `50.00 <ø> (ø)` | |
| #hudiutilities | `70.09% <ø> (ø)` | `327.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r506441500
##########
File path: hudi-integ-test/src/test/resources/unit-test-cow-dag.yaml
##########
@@ -17,23 +17,53 @@ first_insert:
config:
record_size: 70000
num_insert_partitions: 1
- repeat_count: 1
+ repeat_count: 2
Review comment:
my bad, I mistook num records in first_upsert as insert records. just now noticed it as upsert records.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r504823094
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/scheduler/DagScheduler.java
##########
@@ -109,12 +109,16 @@ private void executeNode(DagNode node) {
throw new RuntimeException("DagNode already completed! Cannot re-execute");
}
try {
- log.warn("executing node: " + node.getName() + " of type: " + node.getClass());
- node.execute(executionContext);
- node.setCompleted(true);
- log.info("Finished executing {}", node.getName());
+ int repeatCount = node.getConfig().getRepeatCount();
+ while (repeatCount > 0) {
+ log.warn("executing node: " + node.getName() + " of type: " + node.getClass());
+ node.execute(executionContext);
+ node.setCompleted(true);
Review comment:
You're right, I fixed the repeat count and tested it on the unit tests but didn't confirm the DAG under the demo's. I've changed the repeat count to be 1 in the docker DAG so it doesn't cause confusion and have added repeat_count only to the unit_test dags to confirm repeat count works.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan merged pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan merged pull request #2092:
URL: https://github.com/apache/hudi/pull/2092
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-701149391
@nsivabalan I actually fixed the rollback node as well, need a couple of days to add tests for it after which we can merge it and you will have a fully working DAG with all nodes
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-702767045
cool, sounds good. will wait for you to update the patch then
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r504028617
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/RollbackNode.java
##########
@@ -49,6 +54,11 @@ public void execute(ExecutionContext executionContext) throws Exception {
Option<HoodieInstant> lastInstant = metaClient.getActiveTimeline().getCommitsTimeline().lastInstant();
if (lastInstant.isPresent()) {
log.info("Rolling back last instant {}", lastInstant.get());
+ log.info("Cleaning up generated data for the instant being rolled back {}", lastInstant.get());
+ ValidationUtils.checkArgument(executionContext.getWriterContext().getProps().getOrDefault(DFSPathSelector.Config.SOURCE_INPUT_SELECTOR,
+ DFSPathSelector.class.getName()).toString().equalsIgnoreCase(DFSTestSuitePathSelector.class.getName()), "Test Suite only supports DFSTestSuitePathSelector");
+ metaClient.getFs().delete(new Path(executionContext.getWriterContext().getCfg().inputBasePath,
Review comment:
minor. Can we first rollback from hudi and then delete from input data. Just incase rollback from hudi fails, we will have the input data intact for any debugging.
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/helpers/DFSTestSuitePathSelector.java
##########
@@ -62,19 +67,26 @@ public DFSTestSuitePathSelector(TypedProperties props, Configuration hadoopConf)
lastBatchId = 0;
nextBatchId = 1;
}
-
- log.info("Using DFSTestSuitePathSelector, checkpoint: " + lastCheckpointStr + " sourceLimit: " + sourceLimit
- + " lastBatchId: " + lastBatchId + " nextBatchId: " + nextBatchId);
// obtain all eligible files for the batch
List<FileStatus> eligibleFiles = new ArrayList<>();
FileStatus[] fileStatuses = fs.globStatus(
new Path(props.getString(Config.ROOT_INPUT_PATH_PROP), "*"));
+ // Say input data is as follow input/1, input/2, input/5 since 3,4 was rolled back and 5 is new generated data
+ // checkpoint from the latest commit metadata will be 2 since 3,4 has been rolled back. We need to set the
+ // next batch id correctly as 5 instead of 3
+ Optional<String> correctBatchIdDueToRollback = Arrays.stream(fileStatuses)
+ .map(f -> f.getPath().toString().split("/")[f.getPath().toString().split("/").length - 1])
+ .min((bid1, bid2) -> Integer.min(Integer.parseInt(bid1), Integer.parseInt(bid2)));
Review comment:
sorry, are we not looking for max value here. why min ?
for eg, input/1, input/2, input/5. in this, we are interested in "5" right?
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/scheduler/DagScheduler.java
##########
@@ -109,12 +109,16 @@ private void executeNode(DagNode node) {
throw new RuntimeException("DagNode already completed! Cannot re-execute");
}
try {
- log.warn("executing node: " + node.getName() + " of type: " + node.getClass());
- node.execute(executionContext);
- node.setCompleted(true);
- log.info("Finished executing {}", node.getName());
+ int repeatCount = node.getConfig().getRepeatCount();
+ while (repeatCount > 0) {
+ log.warn("executing node: " + node.getName() + " of type: " + node.getClass());
+ node.execute(executionContext);
+ node.setCompleted(true);
Review comment:
am I missing something here. once the node is set to complete, how can we execute in next cycle if repeatCount > 1 ? shouldn't we set it outside of while loop
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/RollbackNode.java
##########
@@ -49,6 +54,11 @@ public void execute(ExecutionContext executionContext) throws Exception {
Option<HoodieInstant> lastInstant = metaClient.getActiveTimeline().getCommitsTimeline().lastInstant();
if (lastInstant.isPresent()) {
Review comment:
looks like our rollback only supports rolling back the last commit. I assume we need to fix this in some later patch.
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/scheduler/DagScheduler.java
##########
@@ -109,12 +109,16 @@ private void executeNode(DagNode node) {
throw new RuntimeException("DagNode already completed! Cannot re-execute");
}
try {
- log.warn("executing node: " + node.getName() + " of type: " + node.getClass());
- node.execute(executionContext);
- node.setCompleted(true);
- log.info("Finished executing {}", node.getName());
+ int repeatCount = node.getConfig().getRepeatCount();
+ while (repeatCount > 0) {
+ log.warn("executing node: " + node.getName() + " of type: " + node.getClass());
+ node.execute(executionContext);
+ node.setCompleted(true);
Review comment:
in general repeatCount is confusing. for eg:
```
first_insert:
config:
record_size: 70000
num_insert_partitions: 1
repeat_count: 5
num_records_insert: 1000
type: InsertNode
deps: none
```
In this dag yaml, I expect 5*1000 records to be inserted since repeat count is 5. If 5000 records are to be expected, don't we need to fix all yaml dags(complex-dag-mow.yaml etc) for hive query results. I don't see any fixes in this patch for expected record counts though.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash edited a comment on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash edited a comment on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-693160955
@nsivabalan This PR fixes the MOR demo as well, filed another ticket for failing realtime queries (https://issues.apache.org/jira/browse/HUDI-1286). It is un-related to the test-suite but we have to remove the `_rt` queries till that's fixed.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r504152952
##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/RollbackNode.java
##########
@@ -49,6 +54,11 @@ public void execute(ExecutionContext executionContext) throws Exception {
Option<HoodieInstant> lastInstant = metaClient.getActiveTimeline().getCommitsTimeline().lastInstant();
if (lastInstant.isPresent()) {
Review comment:
So, you can add multiple rollbackNode's to simulate multiple commit rollbacks, it will always start from the last node. We still have to implement a RestoreNode that will do this in one shot..
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-699219984
@n3nash : I tested this patch and looks good. once you remove the rollback node, we can merge 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
[GitHub] [hudi] n3nash edited a comment on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash edited a comment on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-693160955
@nsivabalan This PR fixes the MOR demo as well, filed another ticket for failing realtime queries (https://issues.apache.org/jira/browse/HUDI-1286). It is un-related to the test-suite but we have to remove the `*_rt` queries till that's fixed.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r510804292
##########
File path: docker/demo/config/test-suite/complex-dag-cow.yaml
##########
@@ -17,23 +17,23 @@ first_insert:
config:
record_size: 70000
num_insert_partitions: 1
- repeat_count: 5
+ repeat_count: 1
num_records_insert: 1000
type: InsertNode
deps: none
second_insert:
config:
record_size: 70000
num_insert_partitions: 1
- repeat_count: 5
+ repeat_count: 1
num_records_insert: 10000
deps: first_insert
type: InsertNode
third_insert:
config:
record_size: 70000
num_insert_partitions: 1
Review comment:
@n3nash : all dags need to be fixed for partition config.
right ones are
num_partitions_insert
num_partitions_upsert.
where as so far we are using
num_insert_partitions
num_upsert_partitions.
Can you fix these and re-run all your tests and dags. specially around upsert and rollbacks to ensure things are looking good.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r505797510
##########
File path: hudi-integ-test/src/test/resources/unit-test-cow-dag.yaml
##########
@@ -17,23 +17,53 @@ first_insert:
config:
record_size: 70000
num_insert_partitions: 1
- repeat_count: 1
+ repeat_count: 2
Review comment:
We rollback 1 InsertNode, so total 2*100 + 100 - 100 + 100.
##########
File path: hudi-integ-test/src/test/resources/unit-test-cow-dag.yaml
##########
@@ -17,23 +17,53 @@ first_insert:
config:
record_size: 70000
num_insert_partitions: 1
- repeat_count: 1
+ repeat_count: 2
Review comment:
@nsivabalan We rollback 1 InsertNode, so total 2*100 + 100 - 100 + 100.
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-716293969
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2092?src=pr&el=h1) Report
> Merging [#2092](https://codecov.io/gh/apache/hudi/pull/2092?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/49e855c3488ebe53af3a72fc3b5a22ba47a45e01?el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2092?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2092 +/- ##
=========================================
Coverage 53.68% 53.68%
Complexity 2849 2849
=========================================
Files 359 359
Lines 16565 16565
Branches 1782 1782
=========================================
Hits 8893 8893
Misses 6915 6915
Partials 757 757
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (ø)` | `193.00 <ø> (ø)` | |
| #hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| #hudicommon | `54.70% <ø> (ø)` | `1794.00 <ø> (ø)` | |
| #hudihadoopmr | `33.05% <ø> (ø)` | `181.00 <ø> (ø)` | |
| #hudispark | `65.95% <ø> (ø)` | `304.00 <ø> (ø)` | |
| #huditimelineservice | `62.29% <ø> (ø)` | `50.00 <ø> (ø)` | |
| #hudiutilities | `70.09% <ø> (ø)` | `327.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
----------------------------------------------------------------
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
[GitHub] [hudi] n3nash commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-708535130
@nsivabalan addressed 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
[GitHub] [hudi] n3nash commented on pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
n3nash commented on pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#issuecomment-693160955
@nsivabalan This PR fixes the MOR demo as well, filed another ticket for failing realtime queries (https://issues.apache.org/jira/browse/HUDI-1286). It is un-related to the test-suite.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2092: [HUDI-1285] Fix merge on read DAG to make docker demo pass
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2092:
URL: https://github.com/apache/hudi/pull/2092#discussion_r489590017
##########
File path: docker/demo/config/test-suite/complex-dag-mor.yaml
##########
@@ -81,11 +81,7 @@ second_hive_query:
query1: "select count(*) from testdb.table1 group by `_row_key` having count(*) > 1"
result1: 0
query2: "select count(*) from testdb.table1"
- result2: 3100
- query3: "select count(*) from testdb.table1_rt group by `_row_key` having count(*) > 1"
- result3: 0
- query4: "select count(*) from testdb.table1_rt"
- result4: 3100
+ result2: 1100
Review comment:
while you are at it, since rollback is not effective, can we remove it from this dag file until we have a fix. I see this dag more as a sanity check dag and so everything should be succeeding.
##########
File path: docker/demo/config/test-suite/complex-dag-mor.yaml
##########
@@ -81,11 +81,7 @@ second_hive_query:
query1: "select count(*) from testdb.table1 group by `_row_key` having count(*) > 1"
result1: 0
query2: "select count(*) from testdb.table1"
- result2: 3100
- query3: "select count(*) from testdb.table1_rt group by `_row_key` having count(*) > 1"
- result3: 0
- query4: "select count(*) from testdb.table1_rt"
- result4: 3100
+ result2: 1100
Review comment:
same for cow dag as well.
----------------------------------------------------------------
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