mini-lsm
mini-lsm copied to clipboard
skyzh
Added print functionality to iters to allow simpler debugging
What This PR Does
- Adds print functionality to all iterators
- Ensures proper formatting for each iterator
Sample output for a two merge iterator shown below
xxxxxxxxxx TWO_MERGE_ITERATOR xxxxxxxxxx
---------- ITERATOR A ----------
---------- Merge Iterator ----------
Current Iterator (Index 1):
---------- Memtable Iterator ----------
Key : "00"
Value : "2333"
---------- End Memtable Iterator ----------
Iterators in Heap: 1
Iterator Index 0: Valid
---------- Memtable Iterator ----------
Key : "1"
Value : ""
---------- End Memtable Iterator ----------
---------- End Merge Iterator ----------
---------- END ITERATOR A ----------
---------- ITERATOR B ----------
---------- Merge Iterator ----------
Current Iterator (Index 1):
---------- SST_ITERATOR ----------
Current Block Index: 0
---------- Block Iterator ----------
First Key : "0"
Current Key : "0"
Current Value : "2333333"
Key Index : 0
========== Block ==========
Index : 0
Key : 0
Value : 2333333
--------------------
Index : 1
Key : 00
Value : 2333333
--------------------
Index : 2
Key : 4
Value : 23
--------------------
Offsets: [0, 14, 28]
========== End Block ==========
---------- Block Iterator End ----------
---------- SST_ITERATOR_END ----------
Iterators in Heap: 1
Iterator Index 0: Valid
---------- SST_ITERATOR ----------
Current Block Index: 0
---------- Block Iterator ----------
First Key : "4"
Current Key : "4"
Current Value : ""
Key Index : 0
========== Block ==========
Index : 0
Key : 4
Value :
--------------------
Offsets: [0]
========== End Block ==========
---------- Block Iterator End ----------
---------- SST_ITERATOR_END ----------
---------- End Merge Iterator ----------
---------- END ITERATOR B ----------
xxxxxxxxxx END TWO_MERGE_ITERATOR xxxxxxxxxx
Questions
- One point to note is that the decoding scheme used for a block is the compressed key format implemented on week 1 day 7. Should that be the decoding scheme while printing?
- Should this be left as an exercise for the student since they are expected to implement an encoding decoding scheme anyway?
Hello, Is there any particular reason in choosing to add a new method instead of implementing std::fmt::Debug or std::fmt::Display? These traits seems to be a good fit for this use case.
@Adirelle I think the print function has a few advantages
- It's easier to control indentation from parent functions so at some point if we want to improve readability with indentation its easier to do so.
- It feels natural to put it into the iterator trait because every iterator needs to be able to display itself for debugging
What do you recommend?
@redixhumayun Well, std::fmt::Debug seems the idiomatic way of handling debug printing in rust, quoting the std::fmt page : "The purpose of the Debug trait is to facilitate debugging Rust code".
You can ensure that all StorageIterator implementation also implements the Debug trait by adding it as a requirement :
pub trait StorageIterator: std::fmt::Debug {
Your current implementation of print can be used as a base for implementing std::fmt::Debug::fmt, using writeln!(f, ...) instead of println!(...). And, very optionally, you might provide both terse and verbose output, using the # flag to choose which to use.
Once implemented, you can use println! :
println!("{some_iterator:#?}");
My personal preference would be having a Debug implementation for storage iterator. I will take over this pull request and work on that later when I have time :)
I need more time for this pull request :) Thanks anyways and I'll get it merged soon!