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

[IOTDB-3928][IOTDB-4097]New Compaction Performer —— Fast Compaction #7621

Merged
merged 114 commits into from
Nov 26, 2022

Conversation

choubenson
Copy link
Contributor

@choubenson choubenson commented Oct 15, 2022

New Compaction Performer, which speeds up the cross space compaction, including aligned and nonAligned timeseries.

沛辰周 and others added 30 commits July 26, 2022 15:04
…ng sensor schema in fast cross compaction performer, which will cause adding resource read lock more then one time and not releasing it later
… the unseq data has been strengthened to improve the compaction efficiency.
}

@Override
public void excute()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public void excute()
public void execute()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

public void excute()
throws PageException, IllegalPathException, IOException, WriteProcessException {
compactFiles();
if (hasStartMeasurement) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As startNewtMeasurement is an individual method, there should be a method called endMeasurement too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rename to setNewMeasurement.

public class FileElement {
public TsFileResource resource;

public boolean isOverlap = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public boolean isOverlap = false;
public boolean isSelected = false;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

throws IOException, PageException, WriteProcessException, IllegalPathException {
while (!chunkMetadataQueue.isEmpty()) {
ChunkMetadataElement firstChunkMetadataElement = chunkMetadataQueue.peek();
List<ChunkMetadataElement> overlappedChunkMetadatas =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
List<ChunkMetadataElement> overlappedChunkMetadatas =
List<ChunkMetadataElement> overlappedChunkMetadataList =

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.


public long priority;

public boolean isOverlaped = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public boolean isOverlaped = false;
public boolean isSelected= false;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

* <p>Notice: if is aligned chunk, return true if any of value chunk has data been deleted. Return
* false if and only if all value chunks has no data been deleted.
*/
protected boolean isChunkModified(ChunkMetadataElement chunkMetadataElement) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this function to a method of ChunkMetadataElement itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

Comment on lines +311 to +324
while (pointPriorityReader.hasNext()
&& pointPriorityReader.currentPoint().getTimestamp() < nextPageElement.startTime) {
// write data point to chunk writer
compactionWriter.write(pointPriorityReader.currentPoint(), subTaskId);
pointPriorityReader.next();
if (candidateOverlappedPages.size() > oldSize) {
// during the process of writing overlapped points, if the first page is compacted
// completely or a new chunk is deserialized, there may be new pages overlapped with the
// first page in page queue which are added into the list. If so, the next overlapped
// page in the list may be changed, so we should re-get next overlap page here.
oldSize = candidateOverlappedPages.size();
nextPageElement = candidateOverlappedPages.get(0);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The iterator pattern here is not written properly. The correct iterator pattern would call hasNext, then next, and then current, but here it's not that order. So it's easy to think that the last data point is going to be lost here.

Copy link
Member

@qiaojialin qiaojialin left a comment

Choose a reason for hiding this comment

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

Good feature!

@qiaojialin qiaojialin merged commit e0c5e30 into apache:master Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants