HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… by tarak271 · Pull Request #4859 · apache/hive

tarak271 · 2023-11-07T13:39:59Z

…foreEnqueueing fails

What changes were proposed in this pull request?

Change error message thrown when compaction is not successfully initiated

Why are the changes needed?

Regression of enhancement https://issues.apache.org/jira/browse/HIVE-27598

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

Junit tests

zabetak

Shouldn't we remove the @Ignore annotation from
org.apache.hadoop.hive.ql.txn.compactor.TestCrudCompactorOnTez#secondCompactionShouldBeRefusedBeforeEnqueueing to ensure that the changes in this PR fix the problem?

tarak271 · 2023-11-09T07:32:59Z

@zabetak removed @ignore annotation, please help review & merge

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java

sonarqubecloud · 2023-12-04T09:45:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

No Coverage information
No Duplication information

The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java

tarak271 · 2024-01-29T13:14:02Z

@zabetak could you please help guide me, on the next steps with this pull request

zabetak · 2024-02-01T16:19:09Z

Hey @tarak271 apologies for the delay. I think all the comments are addressed so I have to do a careful review of the refactoring to ensure that nothing broke and we should be good to merge this. I see that there are some merge conflicts in the InitiatorBase so if you can resolve these as well that would be helpful.

zabetak

Left a few additional comments for the last refactoring. Please also check the test failures cause they seem kinda of related

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java

zabetak

Some clarifications are still needed for the usage of validTxnList. The other comments are a bunch of nits that I would treat myself if it was just those.

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java

zabetak · 2024-02-28T10:27:29Z

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java

+import org.apache.hadoop.hive.metastore.api.GetValidWriteIdsRequest;
+import org.apache.hadoop.hive.metastore.api.NoSuchTxnException;
+
+


nit: Is there a style guideline that mandates multiple spaces in imports? I don't understand with what criterion they are separated.

formatter did not do it automatically. seems like my mistake

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java

…foreEnqueueing fails

sonarqubecloud · 2024-03-15T05:43:46Z

Quality Gate passed

Issues
19 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

zabetak

LGTM! I will merge this soon.

Many thanks for your persistence and bearing through this long round of reviews.

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java

github-actions bot requested a review from miklosgergely November 7, 2023 13:40

asf-ci-hive added tests pending tests passed and removed tests pending labels Nov 7, 2023

zabetak requested changes Nov 8, 2023

View reviewed changes

github-actions bot requested a review from abstractdog November 8, 2023 13:41

asf-ci-hive added tests pending tests passed and removed tests passed tests pending labels Nov 8, 2023

tarak271 requested a review from zabetak November 9, 2023 07:31

zabetak reviewed Nov 30, 2023

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java Outdated Show resolved Hide resolved

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java Outdated Show resolved Hide resolved

asf-ci-hive added tests pending tests unstable and removed tests passed tests pending tests unstable labels Dec 3, 2023

asf-ci-hive added tests passed and removed tests pending labels Dec 4, 2023

tarak271 requested a review from zabetak December 4, 2023 15:07

zabetak reviewed Dec 8, 2023

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java Outdated Show resolved Hide resolved

tarak271 requested a review from zabetak December 19, 2023 12:08

asf-ci-hive added tests pending and removed tests passed tests pending labels Feb 5, 2024

asf-ci-hive added tests unstable tests pending and removed tests unstable tests pending labels Feb 23, 2024

zabetak requested changes Feb 27, 2024

View reviewed changes

asf-ci-hive added tests pending tests failed and removed tests unstable tests pending tests failed labels Feb 27, 2024

zabetak reviewed Feb 28, 2024

View reviewed changes

zabetak reviewed Mar 11, 2024

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java Show resolved Hide resolved

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java Outdated Show resolved Hide resolved

zabetak requested changes Mar 13, 2024

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java Show resolved Hide resolved

tarak271 and others added 13 commits March 15, 2024 09:39

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

81feab0

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

7c05a97

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

2096d78

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

7fbcc0e

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

b36aa51

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

1e53a87

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

bc1f541

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

04f765b

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

1e86da5

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

43725e9

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

4a96354

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

46f59de

…foreEnqueueing fails

HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe…

ad9a3ce

…foreEnqueueing fails

zabetak approved these changes Mar 19, 2024

View reviewed changes

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java Show resolved Hide resolved

		import org.apache.hadoop.hive.metastore.api.GetValidWriteIdsRequest;
		import org.apache.hadoop.hive.metastore.api.NoSuchTxnException;

Conversation

tarak271 commented Nov 7, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

Is the change a dependency upgrade?

How was this patch tested?

Uh oh!

zabetak left a comment

Choose a reason for hiding this comment

Uh oh!

tarak271 commented Nov 9, 2023

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Dec 4, 2023

Uh oh!

Uh oh!

tarak271 commented Jan 29, 2024

Uh oh!

zabetak commented Feb 1, 2024

Uh oh!

zabetak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zabetak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zabetak Feb 28, 2024

Choose a reason for hiding this comment

Uh oh!

tarak271 Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 15, 2024

Quality Gate passed

Uh oh!

zabetak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants