You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/12/06 06:39:18 UTC
[GitHub] [hive] dengzhhu653 opened a new pull request, #3836: HIVE-23891: UNION ALL and speculative execution can cause file duplication in Tez
dengzhhu653 opened a new pull request, #3836:
URL: https://github.com/apache/hive/pull/3836
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3836: HIVE-23891: UNION ALL and speculative execution can cause file duplication in Tez
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3836:
URL: https://github.com/apache/hive/pull/3836#issuecomment-1338903701
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3836)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3836&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3836&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3836&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3836&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3836&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by GitBox <gi...@apache.org>.
abstractdog commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1066913288
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -162,6 +166,59 @@ public void testNonAcidDynamicPartitioning() throws Exception {
confirmOutput(DataFormat.WITH_PARTITION_VALUE);
}
+ @Test
+ public void testNonAcidRemoveDuplicate() throws Exception {
+ setBasePath("writeDuplicate");
+ setupData(DataFormat.WITH_PARTITION_VALUE);
+
+ FileSinkDesc desc = (FileSinkDesc) getFileSink(AcidUtils.Operation.NOT_ACID, true, 0).getConf().clone();
+ desc.setLinkedFileSink(true);
+ desc.setDirName(new Path(desc.getDirName(), AbstractFileMergeOperator.UNION_SUDBIR_PREFIX + "0"));
+ JobConf jobConf = new JobConf(jc);
+ jobConf.set("hive.execution.engine", "tez");
+ jobConf.set("mapred.task.id", "000000_0");
+ FileSinkOperator op1 = (FileSinkOperator)OperatorFactory.get(new CompilationOpContext(), FileSinkDesc.class);
+ op1.setConf(desc);
+ op1.initialize(jobConf, new ObjectInspector[]{inspector});
+
+ JobConf jobConf2 = new JobConf(jobConf);
+ jobConf2.set("mapred.task.id", "000000_1");
+ FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ new CompilationOpContext(), FileSinkDesc.class);
+ speculative.setConf(desc);
+ speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+
+ for (Object r : rows) {
+ op1.process(r, 0);
+ speculative.process(r, 0);
+ }
+
+ op1.close(false);
+ // speculative task also ends successfully
+ speculative.close(false);
+ Path[] paths = findFilesInBasePath();
+ List<Path> mondays = Arrays.stream(paths)
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 2);
+ Set<String> fileNames = new HashSet<>();
+ fileNames.add(mondays.get(0).getName());
+ fileNames.add(mondays.get(1).getName());
+ Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
Review Comment:
please elaborate on this assert for useful assertion error messages like, like:
```
Assert.assertTrue("000000_1 file is expected", fileNames.contains("000000_1"));
Assert.assertTrue("000000_0 file is expected", fileNames.contains("000000_0"));
```
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -162,6 +166,59 @@ public void testNonAcidDynamicPartitioning() throws Exception {
confirmOutput(DataFormat.WITH_PARTITION_VALUE);
}
+ @Test
+ public void testNonAcidRemoveDuplicate() throws Exception {
+ setBasePath("writeDuplicate");
+ setupData(DataFormat.WITH_PARTITION_VALUE);
+
+ FileSinkDesc desc = (FileSinkDesc) getFileSink(AcidUtils.Operation.NOT_ACID, true, 0).getConf().clone();
+ desc.setLinkedFileSink(true);
+ desc.setDirName(new Path(desc.getDirName(), AbstractFileMergeOperator.UNION_SUDBIR_PREFIX + "0"));
+ JobConf jobConf = new JobConf(jc);
+ jobConf.set("hive.execution.engine", "tez");
+ jobConf.set("mapred.task.id", "000000_0");
+ FileSinkOperator op1 = (FileSinkOperator)OperatorFactory.get(new CompilationOpContext(), FileSinkDesc.class);
+ op1.setConf(desc);
+ op1.initialize(jobConf, new ObjectInspector[]{inspector});
+
+ JobConf jobConf2 = new JobConf(jobConf);
+ jobConf2.set("mapred.task.id", "000000_1");
+ FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ new CompilationOpContext(), FileSinkDesc.class);
+ speculative.setConf(desc);
+ speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+
+ for (Object r : rows) {
+ op1.process(r, 0);
+ speculative.process(r, 0);
+ }
+
+ op1.close(false);
+ // speculative task also ends successfully
+ speculative.close(false);
+ Path[] paths = findFilesInBasePath();
+ List<Path> mondays = Arrays.stream(paths)
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 2);
+ Set<String> fileNames = new HashSet<>();
+ fileNames.add(mondays.get(0).getName());
+ fileNames.add(mondays.get(1).getName());
+ Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
Review Comment:
please elaborate on this assert for useful assertion error messages like:
```
Assert.assertTrue("000000_1 file is expected", fileNames.contains("000000_1"));
Assert.assertTrue("000000_0 file is expected", fileNames.contains("000000_0"));
```
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3836:
URL: https://github.com/apache/hive/pull/3836#issuecomment-1379957242
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3836)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3836&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3836&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3836&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3836&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3836&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3836&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1084142900
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -183,37 +183,42 @@ public void testNonAcidRemoveDuplicate() throws Exception {
JobConf jobConf2 = new JobConf(jobConf);
jobConf2.set("mapred.task.id", "000000_1");
- FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ FileSinkOperator op2 = (FileSinkOperator)OperatorFactory.get(
new CompilationOpContext(), FileSinkDesc.class);
- speculative.setConf(desc);
- speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+ op2.setConf(desc);
+ op2.initialize(jobConf2, new ObjectInspector[]{inspector});
for (Object r : rows) {
op1.process(r, 0);
- speculative.process(r, 0);
+ op2.process(r, 0);
}
op1.close(false);
- // speculative task also ends successfully
- speculative.close(false);
+ // Assume op2 also ends successfully, this happens in different containers
+ op2.close(false);
Path[] paths = findFilesInBasePath();
List<Path> mondays = Arrays.stream(paths)
.filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
.collect(Collectors.toList());
- Assert.assertTrue(mondays.size() == 2);
+ Assert.assertEquals("Two result files were created", 2, mondays.size());
Set<String> fileNames = new HashSet<>();
fileNames.add(mondays.get(0).getName());
fileNames.add(mondays.get(1).getName());
- Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
+ Assert.assertTrue("000000_1 file is expected", fileNames.contains("000000_1"));
+ Assert.assertTrue("000000_0 file is expected", fileNames.contains("000000_0"));
+
+ // This happens in HiveServer2 when the job is finished, the job will call
Review Comment:
yeah, the `closeOp(boolean abort)` is called from task, but the `jobCloseOp` is called from the jobclient side(jobClose-> jobCloseOp):
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java#L732-L748
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by GitBox <gi...@apache.org>.
abstractdog commented on PR #3836:
URL: https://github.com/apache/hive/pull/3836#issuecomment-1378641507
> ### What changes were proposed in this pull request?
> ### Why are the changes needed?
> ### Does this PR introduce _any_ user-facing change?
> ### How was this patch tested?
@dengzhhu653: thanks for the patch, please describe the problem and solution here
I guess the problem is described correctly in jira, so a reader might be interested in the solution description 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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1083273133
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -183,37 +183,42 @@ public void testNonAcidRemoveDuplicate() throws Exception {
JobConf jobConf2 = new JobConf(jobConf);
jobConf2.set("mapred.task.id", "000000_1");
- FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ FileSinkOperator op2 = (FileSinkOperator)OperatorFactory.get(
new CompilationOpContext(), FileSinkDesc.class);
- speculative.setConf(desc);
- speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+ op2.setConf(desc);
+ op2.initialize(jobConf2, new ObjectInspector[]{inspector});
for (Object r : rows) {
op1.process(r, 0);
- speculative.process(r, 0);
+ op2.process(r, 0);
}
op1.close(false);
- // speculative task also ends successfully
- speculative.close(false);
+ // Assume op2 also ends successfully, this happens in different containers
+ op2.close(false);
Path[] paths = findFilesInBasePath();
List<Path> mondays = Arrays.stream(paths)
.filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
.collect(Collectors.toList());
- Assert.assertTrue(mondays.size() == 2);
+ Assert.assertEquals("Two result files were created", 2, mondays.size());
Review Comment:
"Two result files were created" -> makes me think it's unexpected, instead we expect 2 files according to the parameters, can we have rather "2 results files are expected"
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by GitBox <gi...@apache.org>.
abstractdog commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1066917005
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -162,6 +166,59 @@ public void testNonAcidDynamicPartitioning() throws Exception {
confirmOutput(DataFormat.WITH_PARTITION_VALUE);
}
+ @Test
+ public void testNonAcidRemoveDuplicate() throws Exception {
+ setBasePath("writeDuplicate");
+ setupData(DataFormat.WITH_PARTITION_VALUE);
+
+ FileSinkDesc desc = (FileSinkDesc) getFileSink(AcidUtils.Operation.NOT_ACID, true, 0).getConf().clone();
+ desc.setLinkedFileSink(true);
+ desc.setDirName(new Path(desc.getDirName(), AbstractFileMergeOperator.UNION_SUDBIR_PREFIX + "0"));
+ JobConf jobConf = new JobConf(jc);
+ jobConf.set("hive.execution.engine", "tez");
+ jobConf.set("mapred.task.id", "000000_0");
+ FileSinkOperator op1 = (FileSinkOperator)OperatorFactory.get(new CompilationOpContext(), FileSinkDesc.class);
+ op1.setConf(desc);
+ op1.initialize(jobConf, new ObjectInspector[]{inspector});
+
+ JobConf jobConf2 = new JobConf(jobConf);
+ jobConf2.set("mapred.task.id", "000000_1");
+ FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ new CompilationOpContext(), FileSinkDesc.class);
+ speculative.setConf(desc);
+ speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+
+ for (Object r : rows) {
+ op1.process(r, 0);
+ speculative.process(r, 0);
+ }
+
+ op1.close(false);
+ // speculative task also ends successfully
+ speculative.close(false);
+ Path[] paths = findFilesInBasePath();
+ List<Path> mondays = Arrays.stream(paths)
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 2);
+ Set<String> fileNames = new HashSet<>();
+ fileNames.add(mondays.get(0).getName());
+ fileNames.add(mondays.get(1).getName());
+ Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
+
+ op1.jobCloseOp(jobConf, true);
Review Comment:
please comment here on what happens when op1.jobCloseOp is called, I guess this might help in understanding the rest of the assertions happening in this UT
also: is it intentional that the same call has not happened for the speculative task attempt?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by GitBox <gi...@apache.org>.
abstractdog commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1067068061
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -162,6 +166,59 @@ public void testNonAcidDynamicPartitioning() throws Exception {
confirmOutput(DataFormat.WITH_PARTITION_VALUE);
}
+ @Test
+ public void testNonAcidRemoveDuplicate() throws Exception {
+ setBasePath("writeDuplicate");
+ setupData(DataFormat.WITH_PARTITION_VALUE);
+
+ FileSinkDesc desc = (FileSinkDesc) getFileSink(AcidUtils.Operation.NOT_ACID, true, 0).getConf().clone();
+ desc.setLinkedFileSink(true);
+ desc.setDirName(new Path(desc.getDirName(), AbstractFileMergeOperator.UNION_SUDBIR_PREFIX + "0"));
+ JobConf jobConf = new JobConf(jc);
+ jobConf.set("hive.execution.engine", "tez");
+ jobConf.set("mapred.task.id", "000000_0");
+ FileSinkOperator op1 = (FileSinkOperator)OperatorFactory.get(new CompilationOpContext(), FileSinkDesc.class);
+ op1.setConf(desc);
+ op1.initialize(jobConf, new ObjectInspector[]{inspector});
+
+ JobConf jobConf2 = new JobConf(jobConf);
+ jobConf2.set("mapred.task.id", "000000_1");
+ FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ new CompilationOpContext(), FileSinkDesc.class);
+ speculative.setConf(desc);
+ speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+
+ for (Object r : rows) {
+ op1.process(r, 0);
+ speculative.process(r, 0);
+ }
+
+ op1.close(false);
+ // speculative task also ends successfully
+ speculative.close(false);
+ Path[] paths = findFilesInBasePath();
+ List<Path> mondays = Arrays.stream(paths)
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 2);
+ Set<String> fileNames = new HashSet<>();
+ fileNames.add(mondays.get(0).getName());
+ fileNames.add(mondays.get(1).getName());
+ Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
+
+ op1.jobCloseOp(jobConf, true);
+ List<Path> resultFiles = new ArrayList<Path>();
+ recurseOnPath(basePath, basePath.getFileSystem(jc), resultFiles);
+ mondays = resultFiles.stream()
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 1);
Review Comment:
please use Assert.assertEquals which will show the expected vs. actual values
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -162,6 +166,59 @@ public void testNonAcidDynamicPartitioning() throws Exception {
confirmOutput(DataFormat.WITH_PARTITION_VALUE);
}
+ @Test
+ public void testNonAcidRemoveDuplicate() throws Exception {
+ setBasePath("writeDuplicate");
+ setupData(DataFormat.WITH_PARTITION_VALUE);
+
+ FileSinkDesc desc = (FileSinkDesc) getFileSink(AcidUtils.Operation.NOT_ACID, true, 0).getConf().clone();
+ desc.setLinkedFileSink(true);
+ desc.setDirName(new Path(desc.getDirName(), AbstractFileMergeOperator.UNION_SUDBIR_PREFIX + "0"));
+ JobConf jobConf = new JobConf(jc);
+ jobConf.set("hive.execution.engine", "tez");
+ jobConf.set("mapred.task.id", "000000_0");
+ FileSinkOperator op1 = (FileSinkOperator)OperatorFactory.get(new CompilationOpContext(), FileSinkDesc.class);
+ op1.setConf(desc);
+ op1.initialize(jobConf, new ObjectInspector[]{inspector});
+
+ JobConf jobConf2 = new JobConf(jobConf);
+ jobConf2.set("mapred.task.id", "000000_1");
+ FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ new CompilationOpContext(), FileSinkDesc.class);
+ speculative.setConf(desc);
+ speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+
+ for (Object r : rows) {
+ op1.process(r, 0);
+ speculative.process(r, 0);
+ }
+
+ op1.close(false);
+ // speculative task also ends successfully
+ speculative.close(false);
+ Path[] paths = findFilesInBasePath();
+ List<Path> mondays = Arrays.stream(paths)
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 2);
+ Set<String> fileNames = new HashSet<>();
+ fileNames.add(mondays.get(0).getName());
+ fileNames.add(mondays.get(1).getName());
+ Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
+
+ op1.jobCloseOp(jobConf, true);
+ List<Path> resultFiles = new ArrayList<Path>();
+ recurseOnPath(basePath, basePath.getFileSystem(jc), resultFiles);
+ mondays = resultFiles.stream()
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 1);
+ Assert.assertTrue(mondays.get(0).getName().equals("000000_1"));
Review Comment:
similarly, Assert.assertEquals
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1067784634
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -162,6 +166,59 @@ public void testNonAcidDynamicPartitioning() throws Exception {
confirmOutput(DataFormat.WITH_PARTITION_VALUE);
}
+ @Test
+ public void testNonAcidRemoveDuplicate() throws Exception {
+ setBasePath("writeDuplicate");
+ setupData(DataFormat.WITH_PARTITION_VALUE);
+
+ FileSinkDesc desc = (FileSinkDesc) getFileSink(AcidUtils.Operation.NOT_ACID, true, 0).getConf().clone();
+ desc.setLinkedFileSink(true);
+ desc.setDirName(new Path(desc.getDirName(), AbstractFileMergeOperator.UNION_SUDBIR_PREFIX + "0"));
+ JobConf jobConf = new JobConf(jc);
+ jobConf.set("hive.execution.engine", "tez");
+ jobConf.set("mapred.task.id", "000000_0");
+ FileSinkOperator op1 = (FileSinkOperator)OperatorFactory.get(new CompilationOpContext(), FileSinkDesc.class);
+ op1.setConf(desc);
+ op1.initialize(jobConf, new ObjectInspector[]{inspector});
+
+ JobConf jobConf2 = new JobConf(jobConf);
+ jobConf2.set("mapred.task.id", "000000_1");
+ FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ new CompilationOpContext(), FileSinkDesc.class);
+ speculative.setConf(desc);
+ speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+
+ for (Object r : rows) {
+ op1.process(r, 0);
+ speculative.process(r, 0);
+ }
+
+ op1.close(false);
+ // speculative task also ends successfully
+ speculative.close(false);
+ Path[] paths = findFilesInBasePath();
+ List<Path> mondays = Arrays.stream(paths)
+ .filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
+ .collect(Collectors.toList());
+ Assert.assertTrue(mondays.size() == 2);
+ Set<String> fileNames = new HashSet<>();
+ fileNames.add(mondays.get(0).getName());
+ fileNames.add(mondays.get(1).getName());
+ Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog merged pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog merged PR #3836:
URL: https://github.com/apache/hive/pull/3836
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on a diff in pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #3836:
URL: https://github.com/apache/hive/pull/3836#discussion_r1083273355
##########
ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java:
##########
@@ -183,37 +183,42 @@ public void testNonAcidRemoveDuplicate() throws Exception {
JobConf jobConf2 = new JobConf(jobConf);
jobConf2.set("mapred.task.id", "000000_1");
- FileSinkOperator speculative = (FileSinkOperator)OperatorFactory.get(
+ FileSinkOperator op2 = (FileSinkOperator)OperatorFactory.get(
new CompilationOpContext(), FileSinkDesc.class);
- speculative.setConf(desc);
- speculative.initialize(jobConf2, new ObjectInspector[]{inspector});
+ op2.setConf(desc);
+ op2.initialize(jobConf2, new ObjectInspector[]{inspector});
for (Object r : rows) {
op1.process(r, 0);
- speculative.process(r, 0);
+ op2.process(r, 0);
}
op1.close(false);
- // speculative task also ends successfully
- speculative.close(false);
+ // Assume op2 also ends successfully, this happens in different containers
+ op2.close(false);
Path[] paths = findFilesInBasePath();
List<Path> mondays = Arrays.stream(paths)
.filter(path -> path.getParent().toString().endsWith("partval=Monday/HIVE_UNION_SUBDIR_0"))
.collect(Collectors.toList());
- Assert.assertTrue(mondays.size() == 2);
+ Assert.assertEquals("Two result files were created", 2, mondays.size());
Set<String> fileNames = new HashSet<>();
fileNames.add(mondays.get(0).getName());
fileNames.add(mondays.get(1).getName());
- Assert.assertTrue(fileNames.contains("000000_1") && fileNames.contains("000000_0"));
+ Assert.assertTrue("000000_1 file is expected", fileNames.contains("000000_1"));
+ Assert.assertTrue("000000_0 file is expected", fileNames.contains("000000_0"));
+
+ // This happens in HiveServer2 when the job is finished, the job will call
Review Comment:
are you sure it's HiveServer2? as jobCloseOp is part of the operator pipeline, they run as part of the execution in the tasks, am I missing something?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] abstractdog commented on pull request #3836: HIVE-23891: UNION ALL and multiple task attempts can cause file duplication
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on PR #3836:
URL: https://github.com/apache/hive/pull/3836#issuecomment-1399224654
@dengzhhu653: minor comment regarding your latest code comment, otherwise, it's fine, I'm about to approve
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org