Skip to content

Commit

Permalink
Fix TryDeleteKvEntry to return true when delete from file is successful
Browse files Browse the repository at this point in the history
  • Loading branch information
Tinitto committed Nov 7, 2022
1 parent 448993a commit 8e8973b
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 159 deletions.
94 changes: 47 additions & 47 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,58 +220,58 @@ On a average PC

```
cpu: Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
BenchmarkStore_Clear/Clear-8 37508 47812 ns/op
BenchmarkStore_Clear/Clear_with_ttl:_3600-8 36679 33528 ns/op
BenchmarkStore_Compact/Compact-8 1 2060734507 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_hey-8 295 4212383 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_hi-8 296 4168765 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_salut-8 291 4046971 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_bonjour-8 291 4096394 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_hola-8 292 4093013 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_oi-8 291 4068893 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_mulimuta-8 292 4055143 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_hey-8 295 4223834 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_hi-8 294 4043806 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_salut-8 295 4062765 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_bonjour-8 292 4084699 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_hola-8 296 4027317 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_oi-8 295 4029108 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_mulimuta-8 294 4036959 ns/op
BenchmarkStore_GetWithoutTtl/Get_hey-8 1233544 957.2 ns/op
BenchmarkStore_GetWithoutTtl/Get_hi-8 1247316 954.0 ns/op
BenchmarkStore_GetWithoutTtl/Get_salut-8 1235551 958.0 ns/op
BenchmarkStore_GetWithoutTtl/Get_bonjour-8 1231387 966.3 ns/op
BenchmarkStore_GetWithoutTtl/Get_hola-8 1260361 954.3 ns/op
BenchmarkStore_GetWithoutTtl/Get_oi-8 1254897 967.6 ns/op
BenchmarkStore_GetWithoutTtl/Get_mulimuta-8 1253518 954.6 ns/op
BenchmarkStore_GetWithTtl/Get_hey-8 997819 1122 ns/op
BenchmarkStore_GetWithTtl/Get_hi-8 1000000 1431 ns/op
BenchmarkStore_GetWithTtl/Get_salut-8 925994 1334 ns/op
BenchmarkStore_GetWithTtl/Get_bonjour-8 995365 1140 ns/op
BenchmarkStore_GetWithTtl/Get_hola-8 1000000 1111 ns/op
BenchmarkStore_GetWithTtl/Get_oi-8 1000000 1121 ns/op
BenchmarkStore_GetWithTtl/Get_mulimuta-8 994162 1123 ns/op
BenchmarkStore_SetWithoutTtl/Set_hey_English-8 174813 7341 ns/op
BenchmarkStore_SetWithoutTtl/Set_hi_English-8 119372 9074 ns/op
BenchmarkStore_SetWithoutTtl/Set_salut_French-8 122444 9230 ns/op
BenchmarkStore_SetWithoutTtl/Set_bonjour_French-8 128853 9415 ns/op
BenchmarkStore_SetWithoutTtl/Set_hola_Spanish-8 128820 9161 ns/op
BenchmarkStore_SetWithoutTtl/Set_oi_Portuguese-8 124173 9182 ns/op
BenchmarkStore_SetWithoutTtl/Set_mulimuta_Runyoro-8 129810 9204 ns/op
BenchmarkStore_SetWithTtl/Set_hey_English-8 166344 7143 ns/op
BenchmarkStore_SetWithTtl/Set_hi_English-8 110610 9474 ns/op
BenchmarkStore_SetWithTtl/Set_salut_French-8 120817 9377 ns/op
BenchmarkStore_SetWithTtl/Set_bonjour_French-8 126235 9426 ns/op
BenchmarkStore_SetWithTtl/Set_hola_Spanish-8 126758 9440 ns/op
BenchmarkStore_SetWithTtl/Set_oi_Portuguese-8 128312 9403 ns/op
BenchmarkStore_SetWithTtl/Set_mulimuta_Runyoro-8 123814 9273 ns/op
BenchmarkStore_Clear/Clear-8 37342 31261 ns/op
BenchmarkStore_Clear/Clear_with_ttl:_3600-8 37086 31756 ns/op
BenchmarkStore_Compact/Compact-8 1 2070248915 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_hey-8 284754 5521 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_hi-8 185820 6594 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_salut-8 177535 6735 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_bonjour-8 179936 6525 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_hola-8 177618 6735 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_oi-8 173463 6704 ns/op
BenchmarkStore_DeleteWithoutTtl/Delete_key_mulimuta-8 175621 6668 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_hey-8 280477 4010 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_hi-8 307460 6632 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_salut-8 194413 6775 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_bonjour-8 180406 6496 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_hola-8 184796 6556 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_oi-8 178884 6620 ns/op
BenchmarkStore_DeleteWithTtl/Delete_key_mulimuta-8 185463 6597 ns/op
BenchmarkStore_GetWithoutTtl/Get_hey-8 1000000 1024 ns/op
BenchmarkStore_GetWithoutTtl/Get_hi-8 1000000 1031 ns/op
BenchmarkStore_GetWithoutTtl/Get_salut-8 1000000 1032 ns/op
BenchmarkStore_GetWithoutTtl/Get_bonjour-8 996567 1030 ns/op
BenchmarkStore_GetWithoutTtl/Get_hola-8 1131050 1031 ns/op
BenchmarkStore_GetWithoutTtl/Get_oi-8 1000000 1030 ns/op
BenchmarkStore_GetWithoutTtl/Get_mulimuta-8 1000000 1023 ns/op
BenchmarkStore_GetWithTtl/Get_hey-8 989518 1171 ns/op
BenchmarkStore_GetWithTtl/Get_hi-8 990582 1158 ns/op
BenchmarkStore_GetWithTtl/Get_salut-8 961626 1167 ns/op
BenchmarkStore_GetWithTtl/Get_bonjour-8 996882 1168 ns/op
BenchmarkStore_GetWithTtl/Get_hola-8 988996 1162 ns/op
BenchmarkStore_GetWithTtl/Get_oi-8 989503 1165 ns/op
BenchmarkStore_GetWithTtl/Get_mulimuta-8 1000000 1155 ns/op
BenchmarkStore_SetWithoutTtl/Set_hey_English-8 164676 8012 ns/op
BenchmarkStore_SetWithoutTtl/Set_hi_English-8 112162 9356 ns/op
BenchmarkStore_SetWithoutTtl/Set_salut_French-8 127788 9220 ns/op
BenchmarkStore_SetWithoutTtl/Set_bonjour_French-8 129494 9266 ns/op
BenchmarkStore_SetWithoutTtl/Set_hola_Spanish-8 120920 9238 ns/op
BenchmarkStore_SetWithoutTtl/Set_oi_Portuguese-8 123501 9433 ns/op
BenchmarkStore_SetWithoutTtl/Set_mulimuta_Runyoro-8 126140 9325 ns/op
BenchmarkStore_SetWithTtl/Set_hey_English-8 168462 7154 ns/op
BenchmarkStore_SetWithTtl/Set_hi_English-8 126549 9474 ns/op
BenchmarkStore_SetWithTtl/Set_salut_French-8 123495 9541 ns/op
BenchmarkStore_SetWithTtl/Set_bonjour_French-8 118657 9908 ns/op
BenchmarkStore_SetWithTtl/Set_hola_Spanish-8 120117 9700 ns/op
BenchmarkStore_SetWithTtl/Set_oi_Portuguese-8 120649 9792 ns/op
BenchmarkStore_SetWithTtl/Set_mulimuta_Runyoro-8 119839 9718 ns/op
PASS
ok github.com/sopherapps/go-scdb/scdb 71.282s
ok github.com/sopherapps/go-scdb/scdb 62.450s
```

## TODO

- [ ] Optimize `Delete` operation
- [ ] Optimize `Get` operation
- [ ] Optimize the `Compact` operation

## Acknowledgements
Expand Down
108 changes: 54 additions & 54 deletions scdb/internal/buffers/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ type BufferPool struct {
keyValuesStartPoint uint64
maxKeys uint64
redundantBlocks uint16
kvBuffers []Buffer // this will act as a FIFO
indexBuffers []Buffer // this will act as a min-slice where the buffer with the biggest left-offset is replaced
kvBuffers []*Buffer // this will act as a FIFO
indexBuffers map[uint64]*Buffer
File *os.File
FilePath string
FileSize uint64
Expand Down Expand Up @@ -94,8 +94,8 @@ func NewBufferPool(capacity *uint64, filePath string, maxKeys *uint64, redundant
keyValuesStartPoint: header.KeyValuesStartPoint,
maxKeys: header.MaxKeys,
redundantBlocks: header.RedundantBlocks,
kvBuffers: make([]Buffer, 0, kvCap),
indexBuffers: make([]Buffer, 0, indexCap),
kvBuffers: make([]*Buffer, 0, kvCap),
indexBuffers: make(map[uint64]*Buffer, indexCap),
File: file,
FilePath: filePath,
FileSize: fileSize,
Expand All @@ -118,7 +118,7 @@ func (bp *BufferPool) Append(data []byte) (uint64, error) {
start := len(bp.kvBuffers) - 1
for i := start; i >= 0; i-- {
// make sure you get the pointer
buf := &bp.kvBuffers[i]
buf := bp.kvBuffers[i]
if buf.CanAppend(bp.FileSize) {
// write the data to buffer
addr := buf.Append(data)
Expand Down Expand Up @@ -157,13 +157,12 @@ func (bp *BufferPool) UpdateIndex(addr uint64, data []byte) error {
return err
}

for i := 0; i < len(bp.indexBuffers); i++ {
buf := &bp.indexBuffers[i]
if buf.Contains(addr) {
err = buf.Replace(addr, data)
if err != nil {
return err
}
blockLeftOffset := bp.getBlockLeftOffset(addr, entries.HeaderSizeInBytes)
buf, ok := bp.indexBuffers[blockLeftOffset]
if ok {
err = buf.Replace(addr, data)
if err != nil {
return err
}
}

Expand All @@ -180,8 +179,8 @@ func (bp *BufferPool) ClearFile() error {
return err
}
bp.FileSize = uint64(fileSize)
bp.indexBuffers = make([]Buffer, 0, bp.indexCapacity)
bp.kvBuffers = make([]Buffer, 0, bp.kvCapacity)
bp.indexBuffers = make(map[uint64]*Buffer, bp.indexCapacity)
bp.kvBuffers = bp.kvBuffers[:0]
return nil
}

Expand Down Expand Up @@ -272,8 +271,8 @@ func (bp *BufferPool) CompactFile() error {
}

// clean up the buffers and update metadata
bp.kvBuffers = make([]Buffer, 0, bp.kvCapacity)
bp.indexBuffers = make([]Buffer, 0, bp.indexCapacity)
bp.kvBuffers = bp.kvBuffers[:0]
bp.indexBuffers = make(map[uint64]*Buffer, bp.indexCapacity)
bp.File = newFile
bp.FileSize = uint64(newFileOffset)

Expand All @@ -298,7 +297,7 @@ func (bp *BufferPool) GetValue(kvAddress uint64, key []byte) (*Value, error) {
// since the latest kv_buffers are the ones updated when new changes occur
kvBufLen := len(bp.kvBuffers)
for i := kvBufLen - 1; i >= 0; i-- {
buf := &bp.kvBuffers[i]
buf := bp.kvBuffers[i]
if buf.Contains(kvAddress) {
return buf.GetValue(kvAddress, key)
}
Expand All @@ -316,7 +315,7 @@ func (bp *BufferPool) GetValue(kvAddress uint64, key []byte) (*Value, error) {
}

// update kv_buffers only upto actual data read (cater for partially filled buffer)
bp.kvBuffers = append(bp.kvBuffers, *NewBuffer(kvAddress, buf[:bytesRead], bp.bufferSize))
bp.kvBuffers = append(bp.kvBuffers, NewBuffer(kvAddress, buf[:bytesRead], bp.bufferSize))
entry, err := entries.ExtractKeyValueEntryFromByteArray(buf, 0)
if err != nil {
return nil, err
Expand All @@ -338,7 +337,7 @@ func (bp *BufferPool) TryDeleteKvEntry(kvAddress uint64, key []byte) (bool, erro
// since the latest kv_buffers are the ones updated when new changes occur
kvBufLen := len(bp.kvBuffers)
for i := kvBufLen - 1; i >= 0; i-- {
buf := &bp.kvBuffers[i]
buf := bp.kvBuffers[i]
if buf.Contains(kvAddress) {
success, err := buf.TryDeleteKvEntry(kvAddress, key)
if err != nil {
Expand Down Expand Up @@ -367,6 +366,8 @@ func (bp *BufferPool) TryDeleteKvEntry(kvAddress uint64, key []byte) (bool, erro
if err != nil {
return false, err
}

return true, nil
}

return false, nil
Expand All @@ -386,7 +387,7 @@ func (bp *BufferPool) AddrBelongsToKey(kvAddress uint64, key []byte) (bool, erro
// since the latest kv_buffers are the ones updated when new changes occur
kvBufLen := len(bp.kvBuffers)
for i := kvBufLen - 1; i >= 0; i-- {
buf := &bp.kvBuffers[i]
buf := bp.kvBuffers[i]
if buf.Contains(kvAddress) {
return buf.AddrBelongsToKey(kvAddress, key)
}
Expand All @@ -404,7 +405,7 @@ func (bp *BufferPool) AddrBelongsToKey(kvAddress uint64, key []byte) (bool, erro
}

// update kv_buffers only upto actual data read (cater for partially filled buffer)
bp.kvBuffers = append(bp.kvBuffers, *NewBuffer(kvAddress, buf[:bytesRead], bp.bufferSize))
bp.kvBuffers = append(bp.kvBuffers, NewBuffer(kvAddress, buf[:bytesRead], bp.bufferSize))

keyInFile := buf[entries.OffsetForKeyInKVArray : entries.OffsetForKeyInKVArray+uint64(len(key))]
isForKey := bytes.Contains(keyInFile, key)
Expand All @@ -421,48 +422,47 @@ func (bp *BufferPool) ReadIndex(addr uint64) ([]byte, error) {
return nil, err
}

size := entries.IndexEntrySizeInBytes
// starts from buffer with lowest left_offset, which I expect to have more keys
idxBufLen := len(bp.indexBuffers)
for i := 0; i < idxBufLen; i++ {
buf := &bp.indexBuffers[i]
if buf.Contains(addr) {
return buf.ReadAt(addr, size)
}
blockLeftOffset := bp.getBlockLeftOffset(addr, entries.HeaderSizeInBytes)
buf, ok := bp.indexBuffers[blockLeftOffset]
if ok {
return buf.ReadAt(addr, entries.IndexEntrySizeInBytes)
}

buf := make([]byte, bp.bufferSize)
bytesRead, err := bp.File.ReadAt(buf, int64(addr))
data := make([]byte, bp.bufferSize)
// Index buffers should have preset boundaries matching
// StartOfIndex - StartOfIndex + BlockSize,
// StartOfIndex + BlockSize - StartOfIndex + (2*BlockSize)
// StartOfIndex + (2*BlockSize) - StartOfIndex + (3*BlockSize) ...
_, err = bp.File.ReadAt(data, int64(blockLeftOffset))
if err != nil && !errors.Is(err, io.EOF) {
return nil, err
}

// update kv_buffers only upto actual data read (cater for partially filled buffer)
newIdxBuf := NewBuffer(addr, buf[:bytesRead], bp.bufferSize)

if uint64(idxBufLen) >= bp.indexCapacity {
// we wish to remove the last buf, and replace it with the new one
// but maintain the ascending order of LeftOffsets
// we will start at the second-last buf and move towards the front of the slice
for i := idxBufLen - 2; i >= 0; i-- {
currOffset := (&bp.indexBuffers[i]).LeftOffset
if currOffset < newIdxBuf.LeftOffset || i == 0 {
// copy the new buffer in previous position and stop if
// the current buffer has a lower left offset or if we
// have reached the end of the array
bp.indexBuffers[i+1] = *newIdxBuf
break
} else {
// move current buf backwards
bp.indexBuffers[i+1] = bp.indexBuffers[i]
if uint64(len(bp.indexBuffers)) >= bp.indexCapacity {
biggestLeftOffset := uint64(0)
for lftOffset, _ := range bp.indexBuffers {
if lftOffset >= biggestLeftOffset {
biggestLeftOffset = lftOffset
}
}

// delete the buffer with the biggest left offset as those with lower left offsets
// are expected to have more keys
delete(bp.indexBuffers, biggestLeftOffset)
bp.indexBuffers[blockLeftOffset] = NewBuffer(blockLeftOffset, data, bp.bufferSize)
} else {
// Just append
bp.indexBuffers = append(bp.indexBuffers, *newIdxBuf)
bp.indexBuffers[blockLeftOffset] = NewBuffer(blockLeftOffset, data, bp.bufferSize)
}

return buf[:size], nil
start := addr - blockLeftOffset
return data[start : start+entries.IndexEntrySizeInBytes], nil
}

// getBlockLeftOffset returns the left offset for the block in which the address is to be found
func (bp *BufferPool) getBlockLeftOffset(addr uint64, minOffset uint64) uint64 {
blockPosition := (addr - minOffset) / bp.bufferSize
blockLeftOffset := (blockPosition * bp.bufferSize) + minOffset
return blockLeftOffset
}

// Eq checks that other is equal to bp
Expand All @@ -482,13 +482,13 @@ func (bp *BufferPool) Eq(other *BufferPool) bool {
}

for i, buf := range bp.kvBuffers {
if !buf.Eq(&other.kvBuffers[i]) {
if !buf.Eq(other.kvBuffers[i]) {
return false
}
}

for i, buf := range bp.indexBuffers {
if !buf.Eq(&other.indexBuffers[i]) {
if !buf.Eq(other.indexBuffers[i]) {
return false
}
}
Expand Down
Loading

0 comments on commit 8e8973b

Please sign in to comment.