Skip to content

Commit

Permalink
feat(accountsdb): Generate snapshots, fix UAF, improve bincode (#179)
Browse files Browse the repository at this point in the history
* Add utility for identifying arraylist and hashmap

* Make use of less brittle stdlib type check
The string returned by `@typeName` has no guaranteed nor reliable
format, and is not technically guaranteed to be unique to all types.
Using these specific helpers to identify arraylists and hashmaps is
less brittle.

* Update our zstd dependency

* Make the backing int of `FileId` a decl

* Clean up some types & use ArrayHashMap

* Add `writeSnapshotTar` initial implementation

* Make tar also accept EOF in absence of sentinel

* Stop using `@typeName` in `bincode.free` as well
also make it support unmanaged data structures properly

* Improve incremental bank fields & type refactors

* Store slice instead of arraylist in file_map

* LogLevel CLI arg improvement
Remove consequently unused `enumFromName` utility function & amend
some imports in the process.

* Turn the `fields` field into a parameter

* Add utils.fmt.boundedFmt

* Some bincode additions & improvements
* add `arraylist.defaultArrayListUnmanagedOnEOFConfig`
* Handle hashmap size overflow
* Free hashmap key value pair on error
* Error on duplicate hash map entries
* Add `skip_write_fn` config predicate: kind of a duct-tape solution,
  but it'll serve us while we use this bincode model.

* Publicize `ReferenceMemory`

* Implement `writeSnapshotTarTo` method, & more
This commit brings an initial high level implementation for the
generation of snapshots based off of currently derivable data in
accountsdb. It also makes a number of changes to accommodate testing,
and improve code robustness, namely a switch from byte slice paths to
directory handles in related & auxiliary code.
Also this specifically modifies loadAndVerifyAccountsFiles to take
the task indexes in order to slice into the file map data, instead of
the file name list.

Another notable change is the fixup for the `largest_root_slot` field
not actually being assigned the largest available root slot.

There are also various other improvements and refactors.

* Structure testWriteSnapshot better for clean cwd

* bincode improvements & hashmap config
* Make the deserialize allocator parameter non-optional
* Move the free function after the write function
* Add the optional & list namespaces
* Add `readIntAsLength`
* Remove `getSerializedSize`
* Don't return error.EOF instead of error.EndOfStream
* Reorganize struct type reading and writing
* Make `write` use the type detection instead of `@typeName`
* Add dedicated hash map config

* Various accountsdb improvements & fixes
* Pass fields directly instead of passing whole structs
* Store a only a single AccountFileInfo per hashmap entry,
  whilst serializing and deserializing it as a slice.
* Fix copy paste error:
  @"!bincode-config:incremental_snapshot_persistence"
  @"!bincode-config:snapshot_persistence"

* Note probably-temporary parameters & alias types

* Fix UAF with slightly hacky-ish code

* Note purpose of duration of lock in function

* Small renames

* Move tar functions to tar module

* Better snapshot generation names

* run zig fmt

* Rename & assign count value to local variable
* Renames `fields_file_map` to `file_info_map`
* Assign `file_info_map.count()` to `n_account_files`

* Eliminate unused `AccountStorage` type

* Some bincode refactors
* Import `sig.utils.types.arrayListInfo`.
* Clarify control flow.
* Get rid of `MaybeHashMapConfig`.
* Document the `skip_write_fn` predicate.

* Remove blocks

* Bincode simplifications

* Enhance & limit `boundedFmt`

* Correct the account file filtering filtering logic

* Improve file_map iteration & use more `FileId`
* Iterate over `file_info_map` instead of `file_map`, locking only
  account files which are definitely going to be written based on the
  former.
* Concretize the fact that we're only properly supporting full snapshot
  generation at the moment, incremental snapshot generation is future
  work.
* Replace various instances of `id: usize` with `id: FileId` and amend
  related usage sites. In `AccountFileInfo`, this is also amended to
  override the bincode field config to serialize and deserialize it
  as a `usize`.

* Add section for snapshot generation to readme

* Delete dead code, rename local `gen`s to `S`s

* Extract dedicated bincode.readInt function
  • Loading branch information
InKryption committed Jul 16, 2024
1 parent 7bb6345 commit b7e97d5
Show file tree
Hide file tree
Showing 25 changed files with 1,265 additions and 624 deletions.
4 changes: 2 additions & 2 deletions build.zig.zon
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
.hash = "1220b8a918dfcee4fc8326ec337776e2ffd3029511c35f6b96d10aa7be98ca2faf99",
},
.zstd = .{
.url = "https://github.com/Syndica/zstd.zig/archive/a052e839a3dfc44feb00c2eb425815baa3c76e0d.tar.gz",
.hash = "122001d56e43ef94e31243739ae83d7508abf0b8102795aff1ac91446e7ff450d875",
.url = "https://github.com/Syndica/zstd.zig/archive/20a21798a253ea1f0388ee633f4110e1deb2ddef.tar.gz",
.hash = "1220e27e4fece8ff0cabb2f7439891919e8ed294dc936c2a54f0702a2f0ec96f4b6c",
},
.curl = .{
.url = "https://github.com/jiacai2050/zig-curl/archive/8a3f45798a80a5de4c11c6fa44dab8785c421d27.tar.gz",
Expand Down
10 changes: 6 additions & 4 deletions src/accountsdb/accounts_file.zig
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ const AccountFileInfo = @import("snapshots.zig").AccountFileInfo;
/// Simple strictly-typed alias for an integer, used to represent a file ID.
///
/// Analogous to [AccountsFileId](https://github.com/anza-xyz/agave/blob/4c921ca276bbd5997f809dec1dd3937fb06463cc/accounts-db/src/accounts_db.rs#L824)
pub const FileId = enum(u32) {
pub const FileId = enum(Int) {
_,

pub const Int = u32;

pub inline fn fromInt(int: u32) FileId {
return @enumFromInt(int);
}

pub inline fn toInt(file_id: FileId) u32 {
pub inline fn toInt(file_id: FileId) Int {
return @intFromEnum(file_id);
}

Expand Down Expand Up @@ -222,7 +224,7 @@ pub const AccountInFile = struct {
pub const AccountFile = struct {
// file contents
memory: []align(std.mem.page_size) u8,
id: usize,
id: FileId,
slot: Slot,
// number of bytes used
length: usize,
Expand Down Expand Up @@ -395,7 +397,7 @@ test "core.accounts_file: verify accounts file" {
const path = "test_data/test_account_file";
const file = try std.fs.cwd().openFile(path, .{ .mode = .read_write });
const file_info = AccountFileInfo{
.id = 0,
.id = FileId.fromInt(0),
.length = 162224,
};
var accounts_file = try AccountFile.init(file, file_info, 10);
Expand Down
431 changes: 311 additions & 120 deletions src/accountsdb/db.zig

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions src/accountsdb/index.zig
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ pub const AccountRef = struct {
}
};

const ReferenceMemory = std.AutoHashMap(Slot, ArrayList(AccountRef));

/// stores the mapping from Pubkey to the account location (AccountRef)
///
/// Analogous to [AccountsIndex](https://github.com/anza-xyz/agave/blob/a6b2283142192c5360ad0f53bec1eb4a9fb36154/accounts-db/src/accounts_index.rs#L644)
Expand All @@ -79,6 +77,7 @@ pub const AccountIndex = struct {
bins: []RwMux(RefMap),
calculator: PubkeyBinCalculator,

pub const ReferenceMemory = std.AutoHashMap(Slot, ArrayList(AccountRef));
pub const RefMap = SwissMap(Pubkey, AccountReferenceHead, pubkey_hash, pubkey_eql);

const Self = @This();
Expand Down Expand Up @@ -387,7 +386,7 @@ pub const AccountIndex = struct {
.slot = accounts_file.slot,
.location = .{
.File = .{
.file_id = FileId.fromInt(@intCast(accounts_file.id)),
.file_id = accounts_file.id,
.offset = offset,
},
},
Expand Down
19 changes: 17 additions & 2 deletions src/accountsdb/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ loading from a snapshot begins in `accounts_db.loadFromSnapshot` is a very
expensive operation.

the steps include:
- reads and load all the account files
- reads and load all the account files based on the snapshot manifest's file map
- validates + indexes every account in each file (in parallel)
- combines the results across the threads (also in parallel)

Expand Down Expand Up @@ -233,6 +233,21 @@ after validating accounts-db data, we also validate a few key structs:
- `Bank` : contains `bank_fields` which is in the snapshot metadata (not used right now)
- `StatusCache / SlotHistory Sysvar` : additional validation performed in `status_cache.validate`

## generating a snapshot

*note:* at the time of writing, this functionality is in its infancy.

The core logic for generating a snapshot lives in `accounts_db.db.writeSnapshotTarWithFields`; the principle entrypoint is `AccountsDB.writeSnapshotTar`.
The procedure consists of writing the version file, the status cache (`snapshots/status_cache`) file, the snapshot manifest (`snapshots/{SLOT}/{SLOT}`),
and the account files (`accounts/{SLOT}.{FILE_ID}`). This is all written to a stream in the TAR archive format.

The snapshot manifest file content is comprised of the bincoded (bincode-encoded) data structure `SnapshotFields`, which is an aggregate of:
* implicit state: data derived from the current state of AccountsDB, like the file map for all the account which exist at that snapshot, or which have
changed relative to a full snapshot in an incremental one
* configuration state: data that is used to communicate details about the snapshot, like the full slot to which an incremental snapshot is relative.

For full snapshots, we write all account files present in AccountsDB which are rooted - as in, less than or equal to the latest rooted slot.

## read/write benchmarks
`BenchArgs` contains all the configuration of a benchmark (comments describe each parameter)
- found at the bottom of `db.zig`
Expand All @@ -257,4 +272,4 @@ swissmapBenchmark(500k accounts) 1 7715875 7715875 0
WRITE: 17.163ms (1.44x faster than std)
READ: 50.975ms (0.70x faster than std)
swissmapBenchmark(1m accounts) 1 17163500 17163500 0 17163500
```
```
Loading

0 comments on commit b7e97d5

Please sign in to comment.