Skip to content

HIVE-25246: Fix the clean up of open repl created transactions #2396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 17 commits into from
Jul 15, 2021
Merged

HIVE-25246: Fix the clean up of open repl created transactions #2396

merged 17 commits into from
Jul 15, 2021

Conversation

hmangla98
Copy link
Contributor

Fix the clean up of open repl created transactions

pkumarsinha
pkumarsinha reviewed Jun 21, 2021
View reviewed changes
pkumarsinha
pkumarsinha reviewed Jun 21, 2021
View reviewed changes
Copy link
Contributor

@pkumarsinha pkumarsinha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One problem I see is that the txns which are aborted outside replication cycle will lead to a state where when a corresponding commit event come as a part of replication, this will be silently ignored.

pkumarsinha
pkumarsinha reviewed Jun 21, 2021
View reviewed changes
pkumarsinha
pkumarsinha reviewed Jun 24, 2021
View reviewed changes
pkumarsinha
pkumarsinha reviewed Jul 13, 2021
View reviewed changes
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java Outdated
long loadTaskStartTime = System.currentTimeMillis();
SecurityUtils.reloginExpiringKeytabUser();
//Don't proceed if target db is replication incompatible.
if (work.dbNameToLoadIn != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can have some situation in which work.dbNameToLoadIn is not set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know when could that be?

Copy link
Contributor Author

@hmangla98 hmangla98 Jul 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have test cases to test REPL LOAD * scenarios.

pkumarsinha
pkumarsinha reviewed Jul 13, 2021
View reviewed changes
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java Outdated
//Don't proceed if target db is replication incompatible.
if (work.dbNameToLoadIn != null) {
Database targetDb = getHive().getDatabase(work.dbNameToLoadIn);
if (targetDb != null && MetaStoreUtils.isDbReplIncompatible(targetDb)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would targetDb be null? When a wrong work.dbNameToLoadIn is specified ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the first load operation (bootstrap load), targetDb would be null.

pkumarsinha
pkumarsinha reviewed Jul 14, 2021
View reviewed changes
...t/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java Outdated
replica.load("", "`*`");
} catch (SemanticException e) {
assertEquals("REPL LOAD * is not supported", e.getMessage());
assertTrue(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always fail. Why do you need to have such assertion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertEquals("REPL LOAD * is not supported", e.getMessage()); -
Do we not throw this error anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular error is outdated. Earlier, repl load did not raise any exception and hence catch clause was of no use. In this change, Repl load will throw an exception stating that dbname can't be null.

@pkumarsinha pkumarsinha merged commit a75b868 into apache:master Jul 15, 2021
@hmangla98 hmangla98 deleted the HIVE-25246 branch July 15, 2021 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkumarsinha pkumarsinha pkumarsinha approved these changes

Assignees
No one assigned
Labels
tests passed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hmangla98 @pkumarsinha @kgyrtkirk