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

build: add braft format patch #2661

Merged
merged 11 commits into from
Sep 25, 2023
Merged

Conversation

wu-hanqing
Copy link
Contributor

What problem does this PR solve?

Issue Number: #xxx

Problem Summary:

What is changed and how it works?

What's Changed:

How it Works:

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

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@wu-hanqing wu-hanqing force-pushed the build/braft-patch branch 2 times, most recently from 1e545a6 to 22370e8 Compare July 28, 2023 09:52
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

2 similar comments
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@Cyber-SiKu
Copy link
Contributor

@wu-hanqing continue?

@wu-hanqing
Copy link
Contributor Author

cicheck

2 similar comments
@wu-hanqing
Copy link
Contributor Author

cicheck

@YunhuiChen
Copy link
Contributor

cicheck

// Adjust the soft limit of open files to hard limit.
// If |limit| equals 0, then directly return true.
// If hard limit is less than |limit| than return false.
bool AdjustOpenFileSoftLimit(uint64_t limit);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this limit be adjusted dynamically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can use prlimit to change running process's soft and hard limits

Copy link
Contributor

Choose a reason for hiding this comment

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

AdjustOpenFileSoftLimitToHardLimit is better?
Otherwise it looks like setting softlimit to limit

@@ -152,6 +152,14 @@ global.metricDummyServerStartPort=9000
# 是否关闭健康检查: true/关闭 false/不关闭
global.turnOffHealthCheck=true

# minimal open file limit
Copy link
Contributor

Choose a reason for hiding this comment

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

minimal limit? has maximum limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This minimal limit affects how many TCP sockets the client can create, if it's too low, client can't create TCP sockets which will result in an IO Error.

A safe value for this is twice the number of cluster disks

conf/client.conf Outdated
# the number of sockets is related to the number of chunkserver and mds in the cluster,
# and during some exception handling processes, client will create additional sockets
# the SAFE value is 2 * (#chunkserver + #mds)
global.minOpenFileLimit=65536
Copy link
Contributor

Choose a reason for hiding this comment

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

each client 65536?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lowering it to 1024 and it's fine in most cases.

@wu-hanqing
Copy link
Contributor Author

cicheck

1 similar comment
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

3 similar comments
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@@ -274,6 +274,7 @@ struct IOOption {
struct CommonConfigOpt {
bool mdsRegisterToMDS = false;
bool turnOffHealthCheck = false;
uint32_t minimalOpenFiles = 65536;
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the description of the configuration item in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

namespace client {

// Adjust the soft limit of open files to hard limit.
// If |limit| equals 0, then directly return true.
Copy link
Contributor

Choose a reason for hiding this comment

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

limit is uint64_t

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that this code means to set softlimit to hardlimit. But hardlimit has a lower limit called `limit``.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's right.

Copy link
Contributor

@Cyber-SiKu Cyber-SiKu left a comment

Choose a reason for hiding this comment

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

that is all

Copy link
Contributor

@Cyber-SiKu Cyber-SiKu left a comment

Choose a reason for hiding this comment

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

Do you think my understanding is correct?

@wu-hanqing
Copy link
Contributor Author

cicheck

Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
Signed-off-by: Hanqing Wu <wuhanqing@corp.netease.com>
@wu-hanqing
Copy link
Contributor Author

cicheck

7 similar comments
@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing
Copy link
Contributor Author

cicheck

@wu-hanqing wu-hanqing merged commit 4ab72af into opencurve:master Sep 25, 2023
4 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.

None yet

4 participants