Skip to content

add interface to auto clean mysql pendingSegments table#3831

Closed
haoxiang47 wants to merge 5 commits intoapache:masterfrom
haoxiang47:feature-3565
Closed

add interface to auto clean mysql pendingSegments table#3831
haoxiang47 wants to merge 5 commits intoapache:masterfrom
haoxiang47:feature-3565

Conversation

@haoxiang47
Copy link
Copy Markdown

For the issue 3565, I finished the interface to delete unused pendingSeglments, add TaskDataSegment.java to struct the task table’s data, add getNotActiveTask and deletePendingSegments in IndexerSQLMetadataStorageCoordinator.java to finish the delete logic, add test in TestIndexerMetadataStorageCoordinator.java.

…aSegment.java to struct the task table’s data, add getNotActiveTask and deletePendingSegments in IndexerSQLMetadataStorageCoordinator.java to finish the delete logic, add test in TestIndexerMetadataStorageCoordinator.java.
deletePendingSegment’s sql -> “DELETE from %1s WHERE sequence_name LIKE
'%2s%%”

and format other code
…aSegment.java to struct the task table’s data, add getNotActiveTask and deletePendingSegments in IndexerSQLMetadataStorageCoordinator.java to finish the delete logic, add test in TestIndexerMetadataStorageCoordinator.java.
deletePendingSegment’s sql -> “DELETE from %1s WHERE sequence_name LIKE
'%2s%%”

and format other code
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 9, 2017

@KurtYoung can you take a look?

@haoxiang47 haoxiang47 changed the title [Feature 3565]add auto clean mysql pendingSegments table add interface to auto clean mysql pendingSegments table Jan 10, 2017
@@ -0,0 +1,94 @@
package io.druid.timeline;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing license header

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@haoxiang47 there are many whitespace changes, can we please revert them?
https://github.com/druid-io/druid/pull/3831/files?w=1 indicates there's only new code added

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

import java.util.Map;

/**
* Created by haoxiang on 16/11/11.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we generally dont have author info

this.interval = interval;
}

/**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is a weird comment

nuked.addAll(segments);
}

@Override
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

InactiveTasks*

final private Set<DataSegment> published = Sets.newConcurrentHashSet();
final private Set<DataSegment> nuked = Sets.newConcurrentHashSet();
final private List<DataSegment> unusedSegments;
final private List<TaskDataSegment> taskDataSegments;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

inactiveTaskSegments

* @param interval Filter the tasks to ones that include tasks in this interval exclusively. Start is inclusive, end is exclusive
* @return The TaskDataSegment list which used to match the pendingSegment table's payload
*/
List<TaskDataSegment> getNotActiveTask(final Interval interval);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

InactiveTasks

private final Interval interval;

@JsonCreator
public TaskDataSegment(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sure this class is needed? What is the class used for serde when writing to pending Segments table?

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 10, 2017

@gianm is there any possible race condition where active=0 but we still need the segment?

@KurtYoung
Copy link
Copy Markdown
Contributor

KurtYoung commented Jan 11, 2017

@fjy Yeah, i will take a look. @haoxiang47 thanks for your work, and i think it's better to keep the code style consistent with current project, it contains too many spaces and format changes, make it hard to see what exactly had been changed.

@haoxiang47
Copy link
Copy Markdown
Author

Yeah thanks for review, I will keep the code style first and update my code, so excepet the code style, if there has any other problem? My logic will simplely like this:

compare the druid_tasks and druid_pendingSegments, find the dataset which 'active' is 0 in druid_tasks and match the sequence_name in druid_pendingSegments and baseSequenceName in druid_tasks's payload ioConfig parameter which type is kafka.

and then I have the question that where should this interface to action? maybe in overlord or kafka supervisior?

@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Jan 23, 2018

Closing this since #3565 was addressed by #5149

@jon-wei jon-wei closed this Jan 23, 2018
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.

4 participants