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

Chunk pool format asyn #2775

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Chunk pool format asyn #2775

merged 1 commit into from
Oct 24, 2023

Conversation

Vigor-jpg
Copy link
Contributor

What problem does this PR solve?

Issue Number: #2261

Problem Summary:
The time for format chunkfilepool is too long.

What is changed and how it works?

What's Changed:
Make it in backgroud.

How it Works:
Now we start a backgroud thread to format chunkfilepool while we startting chunkserver

Side effects(Breaking backward compatibility? Performance regression?):

Check List

@xu-chaojie xu-chaojie self-requested a review September 25, 2023 10:50
@xu-chaojie
Copy link
Member

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

2 similar comments
@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@@ -199,6 +199,10 @@ chunkfilepool.clean.enable=true
chunkfilepool.clean.bytes_per_write=4096
# The throttle iops for cleaning chunk (4KB/IO)
chunkfilepool.clean.throttle_iops=500
# The size of chunkfilepool
chunkfilepool.chunk_file_pool_size=0
Copy link
Member

Choose a reason for hiding this comment

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

consistent with chunkserver.conf

@@ -89,6 +89,8 @@ message ChunkServerStatisticInfo {
required uint64 chunkSizeTrashedBytes = 7;
// chunkfilepool的大小
optional uint64 chunkFilepoolSize = 8;
// chunkfilepool格式化进度
Copy link
Member

Choose a reason for hiding this comment

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

Use English comments

}

formatStat_.preAllocateNum = needSpace / bytesPerPage;
formatStat_.allocateChunkNum = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a bug here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no problem initializing it to zero. If we need to create a filepool which chunk number is 10, and there are already 3 chunks in this chunkfile pool, then the preAllocateNum is 7, allocateChunkNum is 0.

@@ -192,6 +195,14 @@ int FilePoolHelper::DecodeMetaInfoFromMetaFile(
break;
}

if (!value[kChunkNum].isNull()) {
Copy link
Member

Choose a reason for hiding this comment

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

How to be compatible with meta files of older versions so that they can be upgraded

@Vigor-jpg
Copy link
Contributor Author

cicheck

1 similar comment
@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

1 similar comment
@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

1 similar comment
@Vigor-jpg
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor

CI failed, http://59.111.91.248:8080/job/curve_untest_job/7785/console

you can build and run test locally
image

@Vigor-jpg
Copy link
Contributor Author

cicheck

3 similar comments
@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

1 similar comment
@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

cicheck

1 similar comment
@Vigor-jpg
Copy link
Contributor Author

cicheck

Signed-off-by: yyyyufeng <[email protected]>
@Vigor-jpg
Copy link
Contributor Author

cicheck

1 similar comment
@Vigor-jpg
Copy link
Contributor Author

cicheck

@Vigor-jpg
Copy link
Contributor Author

CI failed, http://59.111.91.248:8080/job/curve_untest_job/7785/console

you can build and run test locally image

This problem is caused by insufficient disk space when we use percentage to allocate chunks in the test environment. It has been fixed now.

@xu-chaojie xu-chaojie merged commit 2009d2e into opencurve:master Oct 24, 2023
5 checks passed
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.

3 participants