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

demux #1150

Merged
merged 9 commits into from
Oct 6, 2021
Merged

demux #1150

merged 9 commits into from
Oct 6, 2021

Conversation

PSeitz
Copy link
Contributor

@PSeitz PSeitz commented Sep 6, 2021

Add support for demux operation (Reorder data from a list of segments to a new list of segments) by leveraging custom delete bitsets on merging.

  • add merge for DeleteBitSet, allow custom DeleteBitSet on merge
  • forward delete bitsets on merge, add tests
  • add demux operation

@PSeitz PSeitz force-pushed the demux branch 2 times, most recently from 5aa717a to c02100e Compare September 6, 2021 11:16
@codecov
Copy link

codecov bot commented Sep 6, 2021

Codecov Report

Merging #1150 (2bc1fc8) into main (ffe4446) will increase coverage by 0.10%.
The diff coverage is 98.82%.

❗ Current head 2bc1fc8 differs from pull request most recent head ce11858. Consider uploading reports for the commit ce11858 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1150      +/-   ##
==========================================
+ Coverage   93.87%   93.98%   +0.10%     
==========================================
  Files         203      204       +1     
  Lines       34010    34534     +524     
==========================================
+ Hits        31928    32456     +528     
+ Misses       2082     2078       -4     
Impacted Files Coverage Δ
src/core/searcher.rs 90.58% <ø> (ø)
src/fastfield/mod.rs 92.30% <ø> (ø)
src/indexer/doc_id_mapping.rs 96.77% <ø> (ø)
src/indexer/mod.rs 100.00% <ø> (ø)
src/lib.rs 98.85% <ø> (ø)
src/indexer/demuxer.rs 98.25% <98.25%> (ø)
src/indexer/segment_updater.rs 94.05% <98.66%> (+1.77%) ⬆️
common/src/bitset.rs 99.31% <100.00%> (+0.85%) ⬆️
src/core/segment_reader.rs 91.66% <100.00%> (+0.58%) ⬆️
src/fastfield/alive_bitset.rs 100.00% <100.00%> (+2.27%) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffe4446...ce11858. Read the comment docs.

@PSeitz
Copy link
Contributor Author

PSeitz commented Sep 9, 2021

Here are some benchmarks:

Merge

time target/release/tantivy merge -i log_testo/
Merge finished with segment meta Tracked(Some(InnerSegmentMeta { segment_id: Seg("af0d919c"), max_doc: 10365151, deletes: None, include_temp_doc_store: true }))
Garbage collect irrelevant segments.

________________________________________________________
Executed in   13,52 secs   fish           external
   usr time   12,03 secs  257,00 micros   12,03 secs
   sys time    0,74 secs  119,00 micros    0,74 secs

Demux into 4 segments, evenly distributed.

pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy demux -i log_testo/ --demux_to log_testo_demuxed/1 --demux_to log_testo_demuxed/2 --demux_to log_testo_demuxed/3 --demux_to log_testo_demuxed/4

________________________________________________________
Executed in   31,18 secs   fish           external
   usr time   28,67 secs  539,00 micros   28,67 secs
   sys time    1,05 secs  235,00 micros    1,05 secs

Demux into 8 segments, evenly distributed.

pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy demux -i log_testo/ --demux_to log_testo_demuxed/1 --demux_to log_testo_demuxed/2 --demux_to log_testo_demuxed/3 --demux_to log_testo_demuxed/4  --demux_to log_testo_demuxed/5  --demux_to log_testo_demuxed/6  --demux_to log_testo_demuxed/7  --demux_to log_testo_demuxed/8

________________________________________________________
Executed in   49,30 secs   fish           external
   usr time   44,74 secs   12,98 millis   44,72 secs
   sys time    1,80 secs    0,06 millis    1,80 secs

Demux to 16

pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> rm -rf log_testo_demuxed;mkdir log_testo_demuxed;mkdir log_testo_demuxed/1;mkdir log_testo_demuxed/2;mkdir log_testo_demuxed/3;mkdir log_testo_demuxed/4;mkdir log_testo_demuxed/5;mkdir log_testo_demuxed/6;mkdir log_testo_demuxed/7;mkdir log_testo_demuxed/8;mkdir log_testo_demuxed/9;mkdir log_testo_demuxed/10;mkdir log_testo_dem
uxed/11;mkdir log_testo_demuxed/12;mkdir log_testo_demuxed/13;mkdir log_testo_demuxed/14;mkdir log_testo_demuxed/15;mkdir log_testo_demuxed/16;
pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy demux -i log_testo/ --demux_to log_testo_demuxed/1 --demux_to log_testo_demuxed/2 --demux_to log_testo_demuxed/3 --demux_to log_testo_demuxed/4  --demux_to log_testo_demuxed/5  --demux_to log_testo_demuxed/6  --demux_to log_testo_demuxed/7  --demux_to log_testo_demuxed/8   --demux_to log_testo_demuxe
d/9   --demux_to log_testo_demuxed/10   --demux_to log_testo_demuxed/11   --demux_to log_testo_demuxed/12   --demux_to log_testo_demuxed/13   --demux_to log_testo_demuxed/14   --demux_to log_testo_demuxed/15   --demux_to log_testo_demuxed/16

________________________________________________________
Executed in   81,28 secs   fish           external
   usr time   73,54 secs    0,00 micros   73,54 secs
   sys time    2,51 secs  1095,00 micros    2,51 secs

pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> du -sh log_testo_demuxed/*
41M     log_testo_demuxed/1
41M     log_testo_demuxed/2
40M     log_testo_demuxed/3
41M     log_testo_demuxed/4
41M     log_testo_demuxed/5
41M     log_testo_demuxed/6
41M     log_testo_demuxed/7
41M     log_testo_demuxed/8
40M     log_testo_demuxed/9
41M     log_testo_demuxed/10
41M     log_testo_demuxed/11
41M     log_testo_demuxed/12
41M     log_testo_demuxed/13
41M     log_testo_demuxed/14
41M     log_testo_demuxed/15
41M     log_testo_demuxed/16
pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> du -sh log_testo/
467M    log_testo/

The time is not linear to the number of segments. With 16 segments, there is an adverse effect of duplicated data, so size of 16 segments is larger than a single segment. That is not yet the case with 4 segments.

@PSeitz
Copy link
Contributor Author

PSeitz commented Sep 10, 2021

Flamegraph of 16x demux. Mostly posting list and fst like in a normal merge. If we want trade speed for memory, parallelization would be really easy.
demux_flamegraph

@fulmicoton
Copy link
Collaborator

I am not sure flamegraph and bench with the fst dictionary make any sense. Demux is a feature that only targets quickwit.

@PSeitz
Copy link
Contributor Author

PSeitz commented Sep 10, 2021

Here are the numbers with the sstable

Merge

pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy merge -i log_testo/
Merge finished with segment meta Tracked(Some(InnerSegmentMeta { segment_id: Seg("b0cda8a4"), max_doc: 10365151, deletes: None, include_temp_doc_store: true }))
Garbage collect irrelevant segments.

________________________________________________________
Executed in    3,09 secs   fish           external
   usr time    2,19 secs  364,00 micros    2,19 secs
   sys time    0,49 secs  214,00 micros    0,49 secs

Demux into 4 segments


pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy demux -i log_testo/ --demux_to log_testo_demuxed/1 --demux_to log_testo_demuxed/2 --demux_to log_testo_demuxed/3 --demux_to log_testo_dem
uxed/4

________________________________________________________
Executed in    9,89 secs   fish           external
   usr time    8,13 secs  262,00 micros    8,13 secs
   sys time    0,78 secs  195,00 micros    0,78 secs

Demux into 8 segments

pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy demux -i log_testo/ --demux_to log_testo_demuxed/1 --demux_to log_testo_demuxed/2 --demux_to log_testo_demuxed/3 --demux_to log_testo_dem
uxed/4  --demux_to log_testo_demuxed/5  --demux_to log_testo_demuxed/6  --demux_to log_testo_demuxed/7  --demux_to log_testo_demuxed/8

________________________________________________________
Executed in   16,08 secs   fish           external
   usr time   13,12 secs  423,00 micros   13,12 secs
   sys time    1,15 secs  312,00 micros    1,15 secs

Demux into 16 segments


pascal@pascal-G533QM ~/L/D/tantivy-cli (main)> time target/release/tantivy demux -i log_testo/ --demux_to log_testo_demuxed/1 --demux_to log_testo_demuxed/2 --demux_to log_testo_demuxed/3 --demux_to log_testo_dem
uxed/4  --demux_to log_testo_demuxed/5  --demux_to log_testo_demuxed/6  --demux_to log_testo_demuxed/7  --demux_to log_testo_demuxed/8   --demux_to log_testo_demuxed/9   --demux_to log_testo_demuxed/10   --demux_
to log_testo_demuxed/11   --demux_to log_testo_demuxed/12   --demux_to log_testo_demuxed/13   --demux_to log_testo_demuxed/14   --demux_to log_testo_demuxed/15   --demux_to log_testo_demuxed/16

________________________________________________________
Executed in   29,43 secs   fish           external
   usr time   23,80 secs  354,00 micros   23,80 secs
   sys time    1,85 secs  207,00 micros    1,85 secs

@PSeitz
Copy link
Contributor Author

PSeitz commented Sep 10, 2021

One hotspot is 10% on is_deleted, which can likely be much improved by having a smarter iterator than:

/// Returns an iterator that will iterate over the alive document ids
pub fn doc_ids_alive(&self) -> impl Iterator<Item = DocId> + '_ {
    (0u32..self.max_doc).filter(move |doc| !self.is_deleted(*doc))
}

hotspots

@PSeitz PSeitz requested a review from fulmicoton September 15, 2021 08:08
src/fastfield/delete.rs Outdated Show resolved Hide resolved
src/fastfield/delete.rs Outdated Show resolved Hide resolved
src/fastfield/delete.rs Outdated Show resolved Hide resolved
src/fastfield/delete.rs Outdated Show resolved Hide resolved
src/fastfield/delete.rs Outdated Show resolved Hide resolved
src/indexer/demuxer.rs Outdated Show resolved Hide resolved
@fulmicoton
Copy link
Collaborator

I like the idea of having a simple&inefficient implementation of Demux, at least for the moment.
This is a good call.

Thank for the bench too. The PR does not seem to be clean enough for review. Please do one quick pass over it.

Also can you put that in tantivy-quickwit instead of tantivy. (The delete bitset work should be in tantivy however)

src/indexer/demuxer.rs Outdated Show resolved Hide resolved
src/indexer/demuxer.rs Outdated Show resolved Hide resolved
src/indexer/demuxer.rs Outdated Show resolved Hide resolved
src/indexer/demuxer.rs Outdated Show resolved Hide resolved
common/src/bitset.rs Outdated Show resolved Hide resolved
common/src/bitset.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@fulmicoton fulmicoton left a comment

Choose a reason for hiding this comment

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

See comments.

@fulmicoton fulmicoton force-pushed the demux branch 3 times, most recently from 45fd78b to 3f4afbc Compare October 6, 2021 06:52
@fulmicoton fulmicoton merged commit 352e0cc into main Oct 6, 2021
@fulmicoton fulmicoton deleted the demux branch October 6, 2021 07:05
This was referenced Feb 18, 2022
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.

2 participants