From d7f9bfdfc521b71b70fc610855d0c389987bf86e Mon Sep 17 00:00:00 2001 From: raphael claude Date: Sun, 20 Aug 2017 01:59:54 +0200 Subject: [PATCH] fix segments sorting in log_merge_policy (#211) bug: segments were sorted on their indices (first field in the tuples) fix: sort on the segments size --- src/indexer/log_merge_policy.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/indexer/log_merge_policy.rs b/src/indexer/log_merge_policy.rs index 47f496998..f565e5ae1 100644 --- a/src/indexer/log_merge_policy.rs +++ b/src/indexer/log_merge_policy.rs @@ -58,7 +58,7 @@ impl MergePolicy for LogMergePolicy { .enumerate() .collect::>(); - size_sorted_tuples.sort_by(|x, y| y.cmp(x)); + size_sorted_tuples.sort_by(|x, y| y.1.cmp(&(x.1))); let size_sorted_log_tuples: Vec<_> = size_sorted_tuples .into_iter() @@ -134,12 +134,21 @@ mod tests { #[test] fn test_log_merge_policy_levels() { // multiple levels all get merged correctly + // 2 MergeCandidates expected: + // * one with the 6 * 10-docs segments + // * one with the 3 * 1000-docs segments + // no MergeCandidate expected for the 2 * 10_000-docs segments as min_merge_size=3 let test_input = vec![seg_meta(10), seg_meta(10), seg_meta(10), seg_meta(1000), seg_meta(1000), - seg_meta(1000)]; + seg_meta(1000), + seg_meta(10000), + seg_meta(10000), + seg_meta(10), + seg_meta(10), + seg_meta(10)]; let result_list = test_merge_policy().compute_merge_candidates(&test_input); assert_eq!(result_list.len(), 2); } @@ -147,18 +156,18 @@ mod tests { #[test] fn test_log_merge_policy_within_levels() { // multiple levels all get merged correctly - let test_input = vec![seg_meta(10), - seg_meta(11), - seg_meta(12), - seg_meta(1000), - seg_meta(1000), - seg_meta(1000)]; + let test_input = vec![seg_meta(10), // log2(10) = ~3.32 (> 3.58 - 0.75) + seg_meta(11), // log2(11) = ~3.46 + seg_meta(12), // log2(12) = ~3.58 + seg_meta(800), // log2(800) = ~9.64 (> 9.97 - 0.75) + seg_meta(1000), // log2(1000) = ~9.97 + seg_meta(1000)]; // log2(1000) = ~9.97 let result_list = test_merge_policy().compute_merge_candidates(&test_input); assert_eq!(result_list.len(), 2); } #[test] fn test_log_merge_policy_small_segments() { - // multiple levels all get merged correctly + // segments under min_layer_size are merged together let test_input = vec![seg_meta(1), seg_meta(1), seg_meta(1),