Skip to content
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.

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

HIVE-17410 : repl load task during subsequent DAG generation does notstart from the last partition processed #240

Closed
wants to merge 2 commits into from

Conversation

anishek
Copy link
Contributor

@anishek anishek commented Aug 30, 2017

No description provided.

tracker.addTask(tasksForAddPartition(table, addPartitionDesc));
ReplicationState currentReplicationState =
new ReplicationState(new PartitionState(table.getTableName(), addPartitionDesc));
updateReplicationState(currentReplicationState);
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to set the replication state only if max number of tasks reached and still have more partitions to load. Or else, need to set it to null if all partitions are added within same DAG. This change is needed for REPL progress log feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Map<String, String> partSpec = addPartitionDesc.getPartition(0).getPartSpec();
Partition ptn = context.hiveDb.getPartition(table, partSpec, false);
if (ptn == null) {
if (!replicationSpec.isMetadataOnly()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

metadataOnly flag is always false in case of bootstrap. So, shall avoid checking it here.

Copy link
Contributor Author

@anishek anishek Sep 8, 2017

Choose a reason for hiding this comment

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

for view it will still be isMetadataOnly though it should not come to partitions load for it, i looked at the code and looks like we can remove a larger part of the code now with this condition not true, will create a bug for it as it will require going through some code branches to figure this out. HIVE-17484

// the destination ptn's repl.last.id is older than the replacement's.
if (replicationSpec.allowReplacementInto(ptn.getParameters())) {
if (replicationSpec.isMetadataOnly()) {
tracker.addTask(alterSinglePartition(addPartitionDesc, replicationSpec, ptn));
Copy link
Contributor

Choose a reason for hiding this comment

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

Dead code as metadata only flag is always false for bootstrap. May be simplify this by safely assuming that "ptn exists" case never occur for bootstrap. Shall add a common method which takes iterator as input and iterates overs the iterator and load partitions. This method can be called by both forNewtable and forExistingTable.

Copy link
Contributor Author

@anishek anishek Sep 8, 2017

Choose a reason for hiding this comment

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

Same as above HIVE-17484

if (replicationSpec.isMetadataOnly()) {
tracker.addTask(alterSinglePartition(addPartitionDesc, replicationSpec, ptn));
if (!tracker.canAddMoreTasks()) {
tracker.setReplicationState(
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is the last partition in the iterator, then should not set the replication state. Shall check this as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this comes under dead code, it will not be executed and will be taken care of as part of HIVE-17484

… start from the last partition processed

setting up the replicationState Correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants