Skip to content

Commit fc784fd

Browse files
authored
commitlog: Change default max-records-in-commit to 1 (#3681)
It is almost always wrong / undesirable to pack more than one transaction in a commit, so adjust the default accordingly. This also avoids surprises when using `#[serde(default)]` with nested structs -- serde evaluates the default depth-first, so overriding a single field in a nested struct will not consider any `#[serde(default = "custom_default")]` annotations on the parent.
1 parent 83fc4cb commit fc784fd

File tree

5 files changed

+48
-8
lines changed

5 files changed

+48
-8
lines changed

crates/commitlog/src/commitlog.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,7 @@ fn range_is_empty(range: &impl RangeBounds<u64>) -> bool {
920920

921921
#[cfg(test)]
922922
mod tests {
923-
use std::{cell::Cell, iter::repeat};
923+
use std::{cell::Cell, iter::repeat, num::NonZeroU16};
924924

925925
use pretty_assertions::assert_matches;
926926

@@ -1231,6 +1231,7 @@ mod tests {
12311231
log.repo.clone(),
12321232
Options {
12331233
max_segment_size: 1024,
1234+
max_records_in_commit: NonZeroU16::new(10).unwrap(),
12341235
..Options::default()
12351236
},
12361237
)

crates/commitlog/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub struct Options {
6262
/// If this number is exceeded, the commit is flushed to disk even without
6363
/// explicitly calling [`Commitlog::flush`].
6464
///
65-
/// Default: 65,535
65+
/// Default: 1
6666
#[cfg_attr(feature = "serde", serde(default = "Options::default_max_records_in_commit"))]
6767
pub max_records_in_commit: NonZeroU16,
6868
/// Whenever at least this many bytes have been written to the currently
@@ -106,7 +106,7 @@ impl Default for Options {
106106

107107
impl Options {
108108
pub const DEFAULT_MAX_SEGMENT_SIZE: u64 = 1024 * 1024 * 1024;
109-
pub const DEFAULT_MAX_RECORDS_IN_COMMIT: NonZeroU16 = NonZeroU16::MAX;
109+
pub const DEFAULT_MAX_RECORDS_IN_COMMIT: NonZeroU16 = NonZeroU16::new(1).expect("1 > 0, qed");
110110
pub const DEFAULT_OFFSET_INDEX_INTERVAL_BYTES: NonZeroU64 = NonZeroU64::new(4096).expect("4096 > 0, qed");
111111
pub const DEFAULT_OFFSET_INDEX_REQUIRE_SEGMENT_FSYNC: bool = false;
112112
pub const DEFAULT_PREALLOCATE_SEGMENTS: bool = false;

crates/commitlog/src/segment.rs

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,16 @@ mod tests {
772772
fn write_read_roundtrip() {
773773
let repo = repo::Memory::unlimited();
774774

775-
let mut writer = repo::create_segment_writer(&repo, Options::default(), Commit::DEFAULT_EPOCH, 0).unwrap();
775+
let mut writer = repo::create_segment_writer(
776+
&repo,
777+
Options {
778+
max_records_in_commit: NonZeroU16::new(4).unwrap(),
779+
..<_>::default()
780+
},
781+
Commit::DEFAULT_EPOCH,
782+
0,
783+
)
784+
.unwrap();
776785
writer.append([0; 32]).unwrap();
777786
writer.append([1; 32]).unwrap();
778787
writer.append([2; 32]).unwrap();
@@ -801,7 +810,16 @@ mod tests {
801810
fn metadata() {
802811
let repo = repo::Memory::unlimited();
803812

804-
let mut writer = repo::create_segment_writer(&repo, Options::default(), Commit::DEFAULT_EPOCH, 0).unwrap();
813+
let mut writer = repo::create_segment_writer(
814+
&repo,
815+
Options {
816+
max_records_in_commit: NonZeroU16::new(3).unwrap(),
817+
..<_>::default()
818+
},
819+
Commit::DEFAULT_EPOCH,
820+
0,
821+
)
822+
.unwrap();
805823
// Commit 0..2
806824
writer.append([0; 32]).unwrap();
807825
writer.append([0; 32]).unwrap();
@@ -835,7 +853,17 @@ mod tests {
835853
let repo = repo::Memory::unlimited();
836854
let commits = vec![vec![[1; 32], [2; 32]], vec![[3; 32]], vec![[4; 32], [5; 32]]];
837855

838-
let mut writer = repo::create_segment_writer(&repo, Options::default(), Commit::DEFAULT_EPOCH, 0).unwrap();
856+
let mut writer = repo::create_segment_writer(
857+
&repo,
858+
Options {
859+
max_records_in_commit: NonZeroU16::new(3).unwrap(),
860+
..<_>::default()
861+
},
862+
Commit::DEFAULT_EPOCH,
863+
0,
864+
)
865+
.unwrap();
866+
839867
for commit in &commits {
840868
for tx in commit {
841869
writer.append(*tx).unwrap();
@@ -868,7 +896,16 @@ mod tests {
868896
let repo = repo::Memory::unlimited();
869897
let commits = vec![vec![[1; 32], [2; 32]], vec![[3; 32]], vec![[4; 32], [5; 32]]];
870898

871-
let mut writer = repo::create_segment_writer(&repo, Options::default(), Commit::DEFAULT_EPOCH, 0).unwrap();
899+
let mut writer = repo::create_segment_writer(
900+
&repo,
901+
Options {
902+
max_records_in_commit: NonZeroU16::new(3).unwrap(),
903+
..<_>::default()
904+
},
905+
Commit::DEFAULT_EPOCH,
906+
0,
907+
)
908+
.unwrap();
872909
for commit in &commits {
873910
for tx in commit {
874911
writer.append(*tx).unwrap();

crates/commitlog/src/tests/helpers.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::fmt::Debug;
1+
use std::{fmt::Debug, num::NonZeroU16};
22

33
use env_logger::Env;
44

@@ -13,6 +13,7 @@ pub fn mem_log<T: Encode>(max_segment_size: u64) -> commitlog::Generic<repo::Mem
1313
repo::Memory::unlimited(),
1414
Options {
1515
max_segment_size,
16+
max_records_in_commit: NonZeroU16::new(10).unwrap(),
1617
..Options::default()
1718
},
1819
)

crates/commitlog/src/tests/partial.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ fn open_log<T>(repo: ShortMem) -> commitlog::Generic<ShortMem, T> {
180180
repo,
181181
Options {
182182
max_segment_size: 1024,
183+
max_records_in_commit: NonZeroU16::new(10).unwrap(),
183184
..Options::default()
184185
},
185186
)

0 commit comments

Comments
 (0)