Commit graph

29 commits

Author SHA1 Message Date
Florian Klink
49b173786c refactor(tvix/castore): remove name from Nodes
Nodes only have names if they're contained inside a Directory, or if
they're a root node and have something else possibly giving them a name
externally.

This removes all `name` fields in the three different Nodes, and instead
maintains it inside a BTreeMap inside the Directory.

It also removes the NamedNode trait (they don't have a get_name()), as
well as Node::rename(self, name), and all [Partial]Ord implementations
for Node (as they don't have names to use for sorting).

The `nodes()`, `directories()`, `files()` iterators inside a `Directory`
now return a tuple of Name and Node, as does the RootNodesProvider.

The different {Directory,File,Symlink}Node struct constructors got
simpler, and the {Directory,File}Node ones became infallible - as
there's no more possibility to represent invalid state.

The proto structs stayed the same - there's now from_name_and_node and
into_name_and_node to convert back and forth between the two `Node`
structs.

Some further cleanups:

The error types for Node validation were renamed. Everything related to
names is now in the DirectoryError (not yet happy about the naming)

There's some leftover cleanups to do:
 - There should be a from_(sorted_)iter and into_iter in Directory, so
   we can construct and deconstruct in one go.
   That should also enable us to implement conversions from and to the
   proto representation that moves, rather than clones.

 - The BuildRequest and PathInfo structs are still proto-based, so we
   still do a bunch of conversions back and forth there (and have some
   ugly expect there). There's not much point for error handling here,
   this will be moved to stricter types in a followup CL.

Change-Id: I7369a8e3a426f44419c349077cb4fcab2044ebb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12205
Tested-by: BuildkiteCI
Reviewed-by: yuka <yuka@yuka.dev>
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: benjaminedwardwebb <benjaminedwardwebb@gmail.com>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-08-17 09:45:58 +00:00
Yureka
3ca0b53840 refactor(tvix/castore): use Directory struct separate from proto one
This uses our own data type to deal with Directories in the castore model.

It makes some undesired states unrepresentable, removing the need for conversions and checking in various places:

 - In the protobuf, blake3 digests could have a wrong length, as proto doesn't know fixed-size fields. We now use `B3Digest`, which makes cloning cheaper, and removes the need to do size-checking everywhere.
 - In the protobuf, we had three different lists for `files`, `symlinks` and `directories`. This was mostly a protobuf size optimization, but made interacting with them a bit awkward. This has now been replaced with a list of enums, and convenience iterators to get various nodes, and add new ones.

Change-Id: I7b92691bb06d77ff3f58a5ccea94a22c16f84f04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12057
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-08-13 12:17:01 +00:00
Yureka
6a988a1598 fix(tvix/castore/GRPCDirectorySvc): fix a bug in the get_recursive fn
When retrieving a closure with get_recursive, the following could happen in the GRPC client:
- The first reference to the deduplicated directory is added to expected_directory_digests
- The deduplicated directory is obtained removed from expected_directory_digests
- The second reference to the deduplicated directory is added to expected_directory_digests
- The deduplicated directory has already been sent, but is still in the
  expected_directory_digests. It looks to the GRPC client like the
  closure is incomplete and the stream ended prematurely.

Change-Id: Ic62bca12e7f8fb85af5fa4dacd199f0f3b8eea8c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12033
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-07-25 09:52:29 +00:00
Yureka
94a0e21e68 fix(tvix/directoryservice): clarify get_recursive not found
Change-Id: I47f21b4b7db9304eadb04094d41bf04d443fcc3b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12025
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: yuka <yuka@yuka.dev>
2024-07-23 14:04:23 +00:00
Yureka
4801d6bf85 feat(tvix): different service types in one composition
This will be necessary for the PathInfoService composition, as some
PathInfoService implementations require a BlobService & DirectoryService
to ingest into.

Using the Extend trait for creating compositions allows extending the same
composition with configs of various types e.g. BlobStore, DirectoryStore
Generics are moved from the Composition struct to the functions.The storage of
the InstantiatonStates uses the TypeId in the key and a Box<dyn Any> in the
value, which is downcasted to InstantiatonState<T>.

Change-Id: I2af11f26c535029adfb1c62905e0e7c4aaed7b51
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11980
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: yuka <yuka@yuka.dev>
2024-07-19 15:13:37 +00:00
Yureka
168e4fda59 refactor(tvix): use composition & registry for from_addr
Change-Id: I3c94ecb5958294b5973c6fcdf5ee9c0d37fa54ad
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11976
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: yuka <yuka@yuka.dev>
2024-07-18 19:19:26 +00:00
Yureka
1a6b6e3ef3 feat(tvix/castore): add composition module
Change-Id: I0868f3278db85ae5fe030089ee9033837bc08748
Signed-off-by: Yureka <tvl@yuka.dev>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11853
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-07-18 10:42:11 +00:00
Florian Klink
7f625e2fc1 refactor(tvix/*store): remove some trait bounds
We don't need to require these things for these impl blocks yet.

Change-Id: I3cec958a637a4f900bdd38abd00e9133bf75ce46
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11865
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Simon Hauser <simon.hauser@helsinki-systems.de>
Tested-by: BuildkiteCI
2024-06-20 21:07:03 +00:00
Simon Hauser
639a00e2ab feat(tvix/tracing): gRPC trace context propagation
This introduces optional helper function in tvix/tracing for trace
propagation and uses these helper in the `tvix-store`.

The GRPCBlobService, GRPCDirectoryService and GRPCPathInfoService now
accept a generic client, meaning the client can be generated with either
`::new` or `::with_interceptor`.

This was tested and validated by starting a `tvix-store daemon` and
`tvix-store import`.

Change-Id: I4b194483bf09266820104b4b56e4a135dca2b77a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11863
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-06-20 19:21:01 +00:00
Simon Hauser
2b20d8d82d feat(tvix/castore): instrument tokio task with current span
By default tokio::spawn does not instrument the spawned task with the
current spawn (https://github.com/tokio-rs/tokio/discussions/6008), do
this manually for all tokio::spawn functions in functions that are
instrumented.

Change-Id: I83dd8145b3a62421454aff57d34180cebbee8304
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11864
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Simon Hauser <simon.hauser@helsinki-systems.de>
2024-06-20 15:48:14 +00:00
Florian Klink
b26569028c feat(tvix/castore/directory/get_recursive): add 'static to BoxStream
Necessary to directly use this in the GRPC DirectoryService wrapper
directly.

Change-Id: Ic6a0038a40dc30071d145af5035345fcd93288ae
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11634
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-13 22:05:59 +00:00
Florian Klink
7bcb896e48 feat(tvix/castore/directory/grpc): instrument functions
Change-Id: I9cc0a6a32184773597556ab5f9250257aa18ca4e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11399
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-04-12 22:32:37 +00:00
Florian Klink
74023a07a4 refactor(tvix/castore/*): drop utils.rs and grpc directorysvc tests
This drops pretty much all of castore/utils.rs.

There were only two things left in there, both a bit messy and only used
for tests:

Some `gen_*_service()` helper functions. These can be expressed by
`from_addr("memory://")`.

The other thing was some plumbing code to test the gRPC layer, by
exposing a in-memory implementation via gRPC, and then connecting to
that channel via a gRPC client again.

Previous CLs moved the connection setup code to
{directory,blob}service::tests::utils, close to where we exercise them,
the new rstest-based tests.

The tests interacting directly on the gRPC types are removed, all
scenarios that were in there show now be covered through the rstest ones
on the trait level.

Change-Id: I450ccccf983b4c62145a25d81c36a40846664814
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11223
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
2024-03-28 07:58:10 +00:00
Florian Klink
345a639e79 refactor(tvix/castore): instrument DirectoryPutter impls consistently
Log the entire span with "trace" level, not just its `ret` level.

The level of the error value event defaults to ERROR, so we don't loose
these.

B3Digest implements Debug and Display the same way, so we can omit the
`(Display)` part in `ret(Display)` for them.

Change-Id: Id00d123a5798e5bdc9820dd97ae2b4d4eb5455f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11218
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-03-20 21:02:44 +00:00
Florian Klink
60b47b336b refactor(tvix/castore/directory): remove GRPCPutter::new
This is no public API to construct this, there's exactly one caller,
and it's perfectly fine to directly populate the struct there.

Change-Id: Idae43a0162ee9bc687d21c550e0c9df33f12d263
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11217
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-03-20 21:02:44 +00:00
Connor Brewster
4e341fb5d9 chore(tvix/store): Use BoxStream type alias
The BoxStream type alias is a more concise and easier to read than
the full `Pin<Box<dyn Stream<Item = ...> + Send + ...>>` type.

Change-Id: I5b7bccfd066ded5557e01f7895f4cf5c4a33bd44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10677
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: Connor Brewster <cbrewster@hey.com>
2024-01-21 19:41:02 +00:00
Florian Klink
96aa220dcf refactor(tvix/castore/directorysvc/grpc/wrapper): no Arc<_>
We can also drop the Clone requirement. Because the trait is async since
some time, there's no need to clone before moving into an async closure,
allowing us to simplify the code a bit.

Change-Id: I9b0a0e10077d8c548d218207b908bfd92c5b8de0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10515
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
2024-01-01 14:40:35 +00:00
Florian Klink
923a5737e6 refactor(tvix/castore): drop is_closed() from impl DirectoryPutter
This is only used in the gRPC version (GRPCPutter), during the test
automation.

So define it as a method there, behind #[cfg(test)], and remove from
the trait.

Change-Id: Idf170884e3a10be0e96c75d946d9c431171e5e88
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10340
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-12-16 23:07:31 +00:00
Florian Klink
8b0047e277 docs(tvix/castore/directorysvc): update comment
This comment didn't make a lot of sense before.

Change-Id: Ie057a133ca4b1a099ed3c885e32316b0d87c5eb0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10339
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-12-13 21:11:58 +00:00
Florian Klink
30e0c32066 refactor(tvix/castore/tonic): make async, support wait-connect=?
This moves the sync `channel::from_url` to a async
`tonic::channel_from_url`. It now allows connecting non-lazily if `wait-
connect=1` is set in the URL params.

Also, make the pingpong tests for blobsvc and directorysvc use the wait-
connect=1 codepath.

Change-Id: Ibeea33117c8121814627e7f6aba0e943ae2e92ca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10030
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-11-15 06:43:31 +00:00
Florian Klink
c83841d3a1 refactor(tvix/castore): remove DirectoryService::from_url
Make directoryservice::from_addr use the more specific constructors.

Change-Id: I9fee2afed77692505988d631d9fe246d9843d25a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10029
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-11-15 06:43:31 +00:00
Florian Klink
9757bf6377 refactor(tvix/*store): helper for channel creation from url
This moves the repetitive code to parse a URL and create a channel
connected to it into `tvix_castore::channel::from_url`.

Part of b/308

Change-Id: Idd342cd71cad5e78a9b258b38c1b227993e75310
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9707
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-10-14 12:26:40 +00:00
Florian Klink
3f011d2762 refactor(tvix/*store): reorganize from_url
Move the channel creation depending on the string-based URL into its
own block.

Change-Id: I546b769acd2296b548eb966b62c495f910266df5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9706
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
2023-10-14 12:26:40 +00:00
Florian Klink
199e5e0339 refactor(tvix/*store): remove some grpc_client let bindings
We had to have these all while the traits where sync, and there was a
lot of spawning and moving.

Most of this can now be removed in favor of some inline `.clone()`.

Change-Id: Id5466c32a403100bc3347866b3172e06a792e311
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9705
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-10-14 12:26:40 +00:00
Florian Klink
dba62157e5 test(tvix/castore): add uds pingpong test for directorysvc
This was missing so far.

Change-Id: Ic8c7781aa0e101342e8a7d94f206937b4163ebfa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9570
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
2023-10-08 22:55:10 +00:00
Florian Klink
b172c804b0 refactor(tvix/castore): use DuplexStream instead of unix socket
We can use DuplexStream to create to bidirectional pairs, which avoids
manually waiting for unix sockets to pop up and connect, and creating
temporary directoires to create the unix sockets in.

Turns out, we also don't actually need to spawn the server in a separate
runtime, it works just fine these days. This might be due to all the
sync barriers in between being gone.

Change-Id: I6b79823bc6209cbcb343b7a498c64a2ba6e0aee7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9562
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2023-10-08 11:25:23 +00:00
Florian Klink
a77914db73 refactor(tvix/castore/directorysvc): factor out gRPC client gen
Move this code into a helper function, which we'll use in other places
in a bit.

Change-Id: Icae6f6dd2d4b2fa86fd2b836ddd7a4ca0e0354e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9559
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
2023-10-08 07:30:34 +00:00
Florian Klink
80a02f96ce chore(tvix/castore): remove dependency on anyhow
We don't use this here, except in a test, where we don't really use
it either.

Change-Id: Ia6c45fccf663fe328942b1e1a2cd1c3a1b7f9ae5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9376
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
2023-09-22 12:51:21 +00:00
Florian Klink
32f41458c0 refactor(tvix): move castore into tvix-castore crate
This splits the pure content-addressed layers from tvix-store into a
`castore` crate, and only leaves PathInfo related things, as well as the
CLI entrypoint in the tvix-store crate.

Notable changes:
 - `fixtures` and `utils` had to be moved out of the `test` cfg, so they
   can be imported from tvix-store.
 - Some ad-hoc fixtures in the test were moved to proper fixtures in the
   same step.
 - The protos are now created by a (more static) recipe in the protos/
   directory.

The (now two) golang targets are commented out, as it's not possible to
update them properly in the same CL. This will be done by a followup CL
once this is merged (and whitby deployed)

Bug: https://b.tvl.fyi/issues/301

Change-Id: I8d675d4bf1fb697eb7d479747c1b1e3635718107
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9370
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-09-22 12:51:21 +00:00
Renamed from tvix/store/src/directoryservice/grpc.rs (Browse further)