Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat(cold-backup): add rate limit for fds #443

Merged
merged 65 commits into from
May 8, 2020

Conversation

levy5307
Copy link
Contributor

@levy5307 levy5307 commented Apr 22, 2020

This pr is intend to add rate limit for fds.
For put operation, we can't send a file in small batches. Because putContent interface of fds will overwrite what was sent before for the same file. So we must send a file all at one batch. It will produce network burst.
But for get operation this problem does not exist, because we get can file content in batches.

New configuration

[replication]
+ fds_write_limit_rate = 100
+ fds_read_limit_rate = 100

@levy5307 levy5307 changed the title feat : add rate limit for cold backup feat: add rate limit for cold backup Apr 22, 2020
@levy5307 levy5307 changed the title feat: add rate limit for cold backup feat: add rate limit for fds Apr 23, 2020
@acelyc111
Copy link
Member

I think we should separte this PR into at least 2 PRs, one to add a new thirdparty lib, and the other to use this lib for coldbackup.

acelyc111
acelyc111 previously approved these changes Apr 30, 2020
src/dist/block_service/fds/fds_service.cpp Outdated Show resolved Hide resolved
"rocksdb_write_buffer_size",
64 * 1024 * 1024,
"rocksdb options.write_buffer_size");
uint64_t max_sst_file_size = std::max(target_file_size, (uint64_t)1.25 * write_buffer_size);
Copy link
Contributor

@neverchanje neverchanje Apr 30, 2020

Choose a reason for hiding this comment

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

rDSN & FDS client should not assume anything about Pegasus, as well as the details around rocksdb. Pegasus is the submodule aka one of the storage engines of rDSN. What if we use other storage engines without rocksdb_write_buffer_size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we should get the file size, or we must set the burst size to a very big number.
Do you have another better way?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's bit of tricky here, but in the following plan, we are going to merge rdsn and pegasus projects.

acelyc111
acelyc111 previously approved these changes Apr 30, 2020
@acelyc111 acelyc111 merged commit 54b4dda into XiaoMi:master May 8, 2020
@neverchanje neverchanje added the type/config-change PR that made modification on configs, which should be noted in release note. label May 14, 2020
@levy5307 levy5307 deleted the backup-rate-limit branch May 26, 2020 06:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2.0.0 type/config-change PR that made modification on configs, which should be noted in release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants