-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Directory structure impl #1879
Directory structure impl #1879
Conversation
77aa734
to
4979700
Compare
89bf1b1
to
3faa3ef
Compare
3faa3ef
to
5be1902
Compare
Some other nits:
|
cb25739
to
598faa2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for the in-depth PR description. That makes review much easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like an improvement to me, though it would be a good time to add some more comments. I also wrote a few unit tests that you could add: noa/directory-structure...jsdt/lock-test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CLI changes LGTM
badf7d6
to
7bcacb3
Compare
7bcacb3
to
20849f8
Compare
20849f8
to
e501984
Compare
Description of Changes
Implements the new directory structure proposal, using strongly-typed newtypes to represent paths. All places in the database/server/cli that previously would have been passing around
PathBuf
s and.join()
ing them now instead pass around these types, calling methods on them that internally call.join()
and return another strongly-typed child node in the directory hierarchy.Changes from the proposal implemented here:
replicas/$database_identity/$replica_id/database/...
, to justreplicas/$replica_id/...
SPACETIMEDB_DISABLE_DISK_LOGGING
,SPACETIMEDB_TRACY
,SPACETIMEDB_FLAMEGRAPH{,_PATH}
) that seem to be just for debugging purposes. I can turn them into command-line args if that's desired.~/.spacetime
directory across the xdg base directories on unix-likes (so~/.config/spacetime
,~/.local/share/spacetime
, etc). On windows it's moved to%LocalAppData%/SpacetimeDB
config.toml
intocli.toml
and data-dirconfig.toml
, and implementeddata-dir/metadata.toml
.data-dir/wasmtime_cache
->data-dir/cache/wasmtime
0.log
(though a proper logrolling mechanism hasn't been implemented - that likely needs further work), database logs are indata-dir/logs
instead of/var/logs/spacetime
.data-dir/spacetime.pid
lockfile, so multiple processes can't use the same data-dir at the same time.cli-{bin-file,bin-dir,config-dir}
,data-dir
) are now bundled in theSpacetimePaths
type. server processes like standalone only interact with the data-dir.spacetime --root-dir
andspacetime --config-path
, to set a custom root directory for the spacetime installation and to specify a customcli.toml
, respectively. this was not in the proposal itself but something like this had to be implemented for our tests to still work.Changes that are not directly related to that, but facilitate it:
spacetimedb::auth::JwtKeys
type, which deduplicates jwt-key-file code across our server implementations. This is something I have less good motivation for, and I could try and revert if desired.Host::try_init
now takes&HostController
, instead of like 5 different args that were just fields onHostController
passed individually. I was adding a 6th arg and clippy was getting mad at me for having too many parameters.Engine
andLinker
are now stored in aWasmtimeRuntime
struct. Since we have to giveEngine
the{data-dir}/cache/wasmtime
path, it made more sense for that to just get eagerly initialized byHostController
, who has access to the data-dir path, instead of threadingdata_dir
all the way through towasmtime::make_actor
.relational_db::test_utils::TempReplicaDir
, which merges aTempDir
andspacetimedb_paths::server::ReplicaDir
.spacetimedb::startup::TracingOptions
got added - this is so we wouldn't read environment variables incore
anymore.