-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
util: ignore computing size if NO_LIMIT #1799
Conversation
src/util/mod.rs
Outdated
let tbls = vec![ | ||
(vec![], NO_LIMIT, 0), | ||
(vec![], size, 0), | ||
(vec![e.clone(); 10], NO_LIMIT, 10), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a case (vec![e.clone(); 10], 0, 1)
.
src/util/mod.rs
Outdated
(vec![e.clone(); 10], size + 1, 1), | ||
(vec![e.clone(); 10], 2 * size , 2), | ||
(vec![e.clone(); 10], 10 * size, 10), | ||
(vec![e.clone(); 10], 10 * size + 1, 10), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a case (vec![e.clone(); 10]; 10 * size - 1, 9)
.
src/raft/raft_log.rs
Outdated
use util; | ||
|
||
pub const NO_LIMIT: u64 = u64::MAX; | ||
pub const NO_LIMIT: u64 = util::NO_LIMIT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why redefine it again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some places use raft NO_LIMIT, I don't want to update them now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you can use pub use util::NO_LIMIT
instead.
PTAL @BusyJay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
src/util/mod.rs
Outdated
return; | ||
pub const NO_LIMIT: u64 = u64::MAX; | ||
|
||
pub fn get_limit_at_size<'a, T: Message + Clone, I: IntoIterator<Item = &'a T>>(entries: I, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use where seems more readable.
LGTM |
LGTM |
Signed-off-by: Ping Yu <[email protected]>
This PR tries to split #1797 and do a little improvement that ignores computing size if NO_LIMIT.
In most cases, we will use NO_LIMIT, so we can return the whole entries directly.
@BusyJay @hhkbp2 @zhangjinpeng1987