fix(level-hash/persistent): new_tail.next points to invalid addres when current tail is removed

This commit is contained in:
Akash Yadav
2024-08-03 09:44:28 +05:30
parent 20ed3ba775
commit ad4dda59f8
2 changed files with 169 additions and 9 deletions
@@ -42,8 +42,9 @@ private val logger = LoggerFactory.getLogger(PersistentLevelHashIO::class.java)
// TODO: When the level hash is expanded, region for the new level in keymap file
// is allocated at the end of the file and the region at the beginning is
// deallocated. When there is enough space at the beginning of the file for the
// new level, we can reuse the space at the beginning of the file. Or, maybe we
// can move the levels towards the beginning of the file when idle.
// new level, we reuse the space at the beginning of the file. Maybe we
// can move the levels towards the beginning of the file when we have enough space
// and the level hash is idle.
// TODO: Allow concurrent access to the level hash.
/**
@@ -157,7 +158,7 @@ private val logger = LoggerFactory.getLogger(PersistentLevelHashIO::class.java)
* - `values_version` - The version of the values file.
* - `keymap_version` - The version of the keymap file.
* - `values_head_entry` - The address of the first entry in the values file.
* - `values_tail_entry` - The address of the next entry in the values file.
* - `values_tail_entry` - The address of the last entry in the values file.
* - `values_file_size_bytes` - The (occupied) size of the values file in bytes.
* - `km_level_size` - The level size of the level hash.
* - `km_bucket_size` - The bucket size of the level hash.
@@ -478,17 +479,25 @@ internal class PersistentLevelHashIO<K : Any, V : Any?>(
}
val current = valuesEntryAt(valueAddr - 1)
if (current.prevEntry == 4486112473) {
print("break")
}
val prev = current.prevEntry // 1-based
val next = current.nextEntry // 1-based
// if this entry has a previous entry, then,
// update the 'next' of previous entry to point to the 'next' of this entry
if (prev > POSITION_INVALID) {
var addr = next
if (next > POSITION_INVALID) {
valIo.position(next - 1)
if (valIo.readInt() <= 0) {
// 'nextEntry' points to an address which is not occupied
// in this case, the 'next' of previous entry should point to this
// (current) entry so the next time an entry will be written, it
// overwrites this region
addr = valueAddr
}
}
valIo.position(prev - 1 + PersistentValueEntry.OFF_NEXT_ENTRY)
valIo.writeLong(next) // 1-based
valIo.writeLong(addr) // 1-based
}
// if this entry has a next entry, then,
@@ -33,6 +33,7 @@ import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import java.io.ByteArrayInputStream
import java.io.File
import kotlin.random.Random
/**
* @author Akash Yadav
@@ -339,7 +340,6 @@ class PersistentLevelHashTest {
assertThat(input.readInt()).isEqualTo(entrySize)
// prevEntry
// + 1 -> prevEntry is 1-based
assertThat(input.readLong()).isEqualTo(prevEntry)
// nextEntry
@@ -359,6 +359,7 @@ class PersistentLevelHashTest {
// value
assertThat(input.readUTF()).isEqualTo("value${i}")
// + 1 -> prevEntry is 1-based
prevEntry = pos + 1
}
}
@@ -366,7 +367,7 @@ class PersistentLevelHashTest {
@Test
fun `test values file binary repr when head is removed`() {
val fileName = "values-binary-repr"
val fileName = "values-binary-repr-rem-head"
persistentStringHash(fileName) {
autoExpand(false)
}.use { hash ->
@@ -440,6 +441,156 @@ class PersistentLevelHashTest {
}
}
@Test
fun `test values file binary repr when tail is removed`() {
val fileName = "values-binary-repr-rem-tail"
persistentStringHash(fileName) {
autoExpand(false)
}.use { hash ->
val entrySize = 38
val count = 10
for (i in 0..<count) {
val key = "key${i}"
val value = "value${i}"
assertThat(hash.insert(key, value)).isTrue()
}
val indexFile =
File("build/level-index/hash-${fileName}/${fileName}.storage")
assertThat(indexFile.exists()).isTrue()
val metaFile =
File("build/level-index/hash-${fileName}/${fileName}.storage._meta")
assertThat(metaFile.exists()).isTrue()
run {
val valIs =
RandomAccessByteArrayInputStream(indexFile.readBytes())
val valIp = ByteStreams.newDataInput(valIs)
val metaIo = PersistentMetaIO(metaFile, hash.levelSize, hash.bucketSize)
assertThat(metaIo.valuesHeadAddr).isEqualTo(1L)
assertThat(metaIo.valuesTailAddr).isEqualTo(((entrySize + 4) * (count - 1)) + 1)
assertThat(valIp.readLong()).isEqualTo(PersistentLevelHashIO.VALUES_MAGIC_NUMBER)
valIs.position((entrySize + 4L) * (count - 1) + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(entrySize)
assertThat(valIp.readLong()).isEqualTo(((entrySize + 4) * (count - 2)) + 1)
// 'nextEntry' of last entry points to the location where new entry
// should be added
assertThat(valIp.readLong()).isEqualTo((entrySize + 4L) * count + 1)
}
// remove head
hash.remove("key9")
run {
val valIs =
RandomAccessByteArrayInputStream(indexFile.readBytes())
val valIp = ByteStreams.newDataInput(valIs)
val metaIo = PersistentMetaIO(metaFile, hash.levelSize, hash.bucketSize)
assertThat(metaIo.valuesHeadAddr).isEqualTo(1L)
assertThat(metaIo.valuesTailAddr).isEqualTo(((entrySize + 4) * (count - 2)) + 1)
assertThat(valIp.readLong()).isEqualTo(PersistentLevelHashIO.VALUES_MAGIC_NUMBER)
valIs.position((entrySize + 4L) * (count - 1) + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(0) // no 'key9' entry as it was removed
valIs.position((entrySize + 4L) * (count - 2) + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(entrySize) // tail entry is 'key8'
assertThat(valIp.readLong()).isEqualTo(((entrySize + 4) * (count - 3)) + 1)
// 'nextEntry' of last entry points to the location where new entry
// should be added
assertThat(valIp.readLong()).isEqualTo((entrySize + 4L) * (count - 1) + 1)
}
}
}
@Test
fun `test values file binary repr when entry is removed at random pos in the middle`() {
val fileName = "values-binary-repr-rem-tail"
persistentStringHash(fileName) {
autoExpand(false)
}.use { hash ->
val entrySize = 38
val count = 10
for (i in 0..<count) {
val key = "key${i}"
val value = "value${i}"
assertThat(hash.insert(key, value)).isTrue()
}
val indexFile =
File("build/level-index/hash-${fileName}/${fileName}.storage")
assertThat(indexFile.exists()).isTrue()
val metaFile =
File("build/level-index/hash-${fileName}/${fileName}.storage._meta")
assertThat(metaFile.exists()).isTrue()
val toRemoveIdx = Random.nextInt(1, count - 1)
run {
val valIs =
RandomAccessByteArrayInputStream(indexFile.readBytes())
val valIp = ByteStreams.newDataInput(valIs)
val metaIo = PersistentMetaIO(metaFile, hash.levelSize, hash.bucketSize)
assertThat(metaIo.valuesHeadAddr).isEqualTo(1L)
assertThat(metaIo.valuesTailAddr).isEqualTo(((entrySize + 4) * (count - 1)) + 1)
assertThat(valIp.readLong()).isEqualTo(PersistentLevelHashIO.VALUES_MAGIC_NUMBER)
valIs.position((entrySize + 4L) * toRemoveIdx + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(entrySize)
assertThat(valIp.readLong()).isEqualTo(((entrySize + 4) * (toRemoveIdx - 1)) + 1)
assertThat(valIp.readLong()).isEqualTo((entrySize + 4L) * (toRemoveIdx + 1) + 1)
assertThat(valIp.readInt()).isEqualTo(6)
assertThat(valIp.readUTF()).isEqualTo("key${toRemoveIdx}")
}
hash.remove("key${toRemoveIdx}")
run {
val valIs =
RandomAccessByteArrayInputStream(indexFile.readBytes())
val valIp = ByteStreams.newDataInput(valIs)
val metaIo = PersistentMetaIO(metaFile, hash.levelSize, hash.bucketSize)
assertThat(metaIo.valuesHeadAddr).isEqualTo(1L)
assertThat(metaIo.valuesTailAddr).isEqualTo(((entrySize + 4) * (count - 1)) + 1)
assertThat(valIp.readLong()).isEqualTo(PersistentLevelHashIO.VALUES_MAGIC_NUMBER)
valIs.position((entrySize + 4L) * toRemoveIdx + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(0) // no 'key9' entry as it was removed
valIs.position((entrySize + 4L) * (toRemoveIdx - 1) + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(entrySize)
assertThat(valIp.readLong()).isEqualTo(((entrySize + 4) * (toRemoveIdx - 2)) + 1)
assertThat(valIp.readLong()).isEqualTo((entrySize + 4L) * toRemoveIdx + 1)
assertThat(valIp.readInt()).isEqualTo(6)
assertThat(valIp.readUTF()).isEqualTo("key${toRemoveIdx - 1}")
valIs.position((entrySize + 4L) * (toRemoveIdx + 1) + 8L) // +8 -> size of magic number
assertThat(valIp.readInt()).isEqualTo(entrySize)
assertThat(valIp.readLong()).isEqualTo(((entrySize + 4) * (toRemoveIdx - 1)) + 1)
assertThat(valIp.readLong()).isEqualTo((entrySize + 4L) * toRemoveIdx + 2)
assertThat(valIp.readInt()).isEqualTo(6)
assertThat(valIp.readUTF()).isEqualTo("key${toRemoveIdx + 1}")
}
}
}
private class RandomAccessByteArrayInputStream(
buf: ByteArray,
pos: Int = 0,