Skip to content

Add level merge to "merge" command#2605

Merged
HTHou merged 53 commits intoapache:masterfrom
zhanglingzhe0820:replace_merge_cli_by_compaction_cli
Feb 22, 2021
Merged

Add level merge to "merge" command#2605
HTHou merged 53 commits intoapache:masterfrom
zhanglingzhe0820:replace_merge_cli_by_compaction_cli

Conversation

@zhanglingzhe0820
Copy link
Contributor

@zhanglingzhe0820 zhanglingzhe0820 commented Jan 29, 2021

Currently, the cli merge can just execute unseq compaction, it is not conveniently for user to execute compaction. So in this PR, I change the merge command to execute both level compaction and unseq merge. And also, this PR may fix the 'file not found' ci problem.
There is a windows ci problem now.
image
Every time this ci problem occurs, there will be a file not found error and it is all windows ci. Although the wrong test was not manually merged, I guessed that it was because the last test reported 'file not found' error affected this test
So I speculate that it is caused by an unseq compaction problem.
image
This error happens because we submit an unseq compaction task manually when a level compaction is working, the unseq compaction will get the current tsfile list and wait for the level compaction to be finished. However, the level compaction may merge the file which will be used in the unseq compaction process. The unseq compaction then cannot find the file to use. Then cause the ci problem.
So in this pr, we change the function of merge cli from the execution of unseq compaction to the execution of level compaction+unseq compaction. It just starts a normal compaction process which do not have this file-related problem.

zhanglingzhe0820 and others added 30 commits November 6, 2020 19:30
@zhanglingzhe0820 zhanglingzhe0820 changed the title Replace merge cli by compaction cli Add level merge to "merge" command Jan 31, 2021
Copy link
Contributor

@jt2594838 jt2594838 left a comment

Choose a reason for hiding this comment

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

How you intend to solve the problem is a bit out of my expectation. Removing current locks may introduce other issues potentially.
I think the problem is actually one merge does not know that another merge is compacting its chosen files. Can we avoid this from the beginning somehow? For example, a merging mark for those files which are already chosen.

@jixuan1989
Copy link
Member

However, the level compaction may merge the file which will be used in the unseq compaction process. The unseq compaction then cannot find the file to use. Then cause the ci problem.

Why it affects query?

@zhanglingzhe0820
Copy link
Contributor Author

However, the level compaction may merge the file which will be used in the unseq compaction process. The unseq compaction then cannot find the file to use. Then cause the ci problem.

Why it affects query?

Every time this ci problem occurs, there will be a file not found error and it is all windows ci. Although the wrong test was not manually merged, I guessed that it was because the last test reported 'file not found' error affected this test
So I speculate that it is caused by an unseq compaction problem.
And Why it affects query?
When compaction meets error, it may destroy the current tearDown() process and leave some file that not be deleted. The data of these remaining files conflicts with the data inserted in the next test, resulting in query errors

@jixuan1989 jixuan1989 added 0.11.3 bug Something isn't working labels Feb 2, 2021
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 5, 2021

@HTHou HTHou merged commit 67b08f9 into apache:master Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants