# VarTree & VarNode — Review (Hy3 preview)

> **Date**: 2026-05-15
> **Scope**: `armdb/docs/design.md` + `armdb/src/var_tree.rs`

## Общее мнение

Дизайн выглядит продуманным и ориентированным на высокую производительность (SkipList на `seize`, block cache, отсутствие аллокаций в hot path). Разделение на `ConstTree` (только in-memory) и `VarTree` (in-memory + disk) — правильное архитектурное решение.

Однако в реализации `VarTree` (и связанных структурах) есть несколько мест, требующих внимания.

## Потенциальные ошибки и проблемные места

### 1. Отсутствие `VarNode` в коде `var_tree.rs`

В дизайне (`design.md`, L148-154) описана структура `VarNode`:

```rust
struct VarNode {
    key: K,
    disk: DiskLoc,
    height: u8,
    tower_ptr: ...
}
```

В файле `var_tree.rs` нет определения `VarNode`. Судя по коду `VarTree`, поле `index` и работа с узлами скрыты внутри модуля индекса (вероятно, `crate::index::SkipList`).

**Рекомендация**: Если `VarNode` реализован в другом месте, убедись, что он корректно работает с `DiskLoc` и `tower_ptr`, и что обновления `DiskLoc` (например, при записи на диск) происходят атомарно или под защитой `write_lock`.

### 2. Логика `get` и Tombstones (L194-212)

В методе `get` есть проверка на тумбстоун:

```rust
if node.disk.is_tombstone() {
    return Ok(None);
}
```

**Проблема**: Если `DiskLoc` помечен как тумбстоун, но в кэше (`self.cache`) еще лежит старое значение, метод вернет `None` (что правильно), но кэш продолжит хранить мусор.

**Рекомендация**: В дизайне упоминается `invalidate_blocks`, но не видно явной инвалидации кэша значений при удалении в `delete` или `cas`. Стоит добавить очистку кэша при обнаружении тумбстоуна или при выполнении delete-операций.

### 3. `cas` и Sequence Numbers (L824-864)

В `cas` используется логика сравнения `current_sequence`:

```rust
let current_seq = node.map(|n| n.disk.sequence());
if current_seq != old_seq {
    return Ok(Err(CasError::Conflict { ... }));
}
```

**Риск**: Если `old_seq` — это `None` (ожидаем отсутствие ключа), а на диске лежит тумбстоун (который имеет валидный sequence), логика может повести себя некорректно.

**Рекомендация**: Нужно убедиться, что тумбстоун считается "отсутствием" ключа для CAS-операций, если это предусмотрено дизайном. Явно протестируй сценарий: `cas(key, None, Some(val))`, когда на диске уже есть тумбстоун для этого ключа.

### 4. Шардирование и `shard_for` (L972-992)

```rust
fn shard_for(&self, key: &K) -> usize {
    let hash = key.hash(); // предполагаем наличие метода hash
    (hash >> (64 - self.shard_prefix_bits)) as usize
}
```

**Проблема**: Если `shard_prefix_bits` равен 0 или `shard_count` не является степенью двойки, маска `(1 << shard_prefix_bits) - 1` может работать некорректно. В дизайне (L677-684) указано, что `shard_count` должен быть степенью двойки, но в коде `VarTree::open_inner` не видно проверки этого инварианта.

**Рекомендация**: Добавь `assert!(self.shard_count.is_power_of_two())` в `open_inner` или соответствующую валидацию в `Config`.

### 5. Итераторы и Weak Consistency (L1177-1259)

Итератор `VarIter` заявлен как weakly-consistent. В `next` и `next_back` происходит резолвинг значений через `resolve_front_asc/rev`.

**Проблема**: Если во время итерации происходит compaction, `DiskLoc` может стать невалидным (ссылка на старый файл). В коде не видно механизма защиты от этого (например, через `Arc` на файл или `epoch`-based защиту), кроме как в самом `engine`.

**Рекомендация**: Убедиться, что `engine` гарантирует валидность `DiskLoc` на время жизни итератора, или использовать `Arc<ActiveFile>` / `Arc<ImmutableFile>` внутри `DiskLoc`.

### 6. `ByteView` и Drop (из design.md)

В дизайне (L336-341) `ByteView::drop` уменьшает `ref_count`.

**Риск**: Если `ByteView` будет отправлен в другой поток (так как `VarTree` реализует `Send`), а `ref_count` не атомарный, возможно UB. Судя по дизайну, `ref_count` должен быть атомарным (`AtomicUsize`).

**Рекомендация**: Проверить, что `ByteView` корректно реализует `Send + Sync` и что `ref_count` — атомик.

### 7. Обработка ошибок в `read_value_cached` (L726-761)

Метод возвращает `GeneralResult<Option<ByteView>>`.

**Проблема**: Если чтение с диска упало по ошибке (IO error), эта ошибка может быть проигнорирована на верхнем уровне, если вызывающий код просто делает `ok().flatten()`.

**Рекомендация**: Проверить, как обрабатываются ошибки чтения в `get` и итераторах. Возможно, стоит разделить `Result` на `Ok(Some)` / `Ok(None)` / `Err(io_error)`.

## Итог

Реализация `VarTree` следует дизайну, но требует доработки в части:
1. Инвалидации кэша при удалении/тумбстоунах.
2. Корректной обработки тумбстоунов в CAS.
3. Валидации параметров шардирования.
4. Безопасности итераторов при compaction.

Большинство проблем — это edge cases, которые проявятся под нагрузкой или при специфических паттернах использования (CAS, удаления, репликация).
