)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":true,"context_lines":[{"line_number":19,"context_line":"   async (like the gRPC server)"},{"line_number":20,"context_line":" - We had to write our own custom glue code (SyncReadIntoAsyncRead)"},{"line_number":21,"context_line":"   to convert a sync io::Read into a tokio::io::AsyncRead, which also"},{"line_number":22,"context_line":"   seemed to be hard to upstream."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This now makes the BlobService trait async (via the async_trait macro,"},{"line_number":25,"context_line":"like we already do in various gRPC parts), and replaces the sync readers"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"f96afd8a_e6e4ea6b","line":22,"updated":"2023-09-15 20:11:00.000000000","message":"Not totally true, we discovered they had it internally but they didn\u0027t want to expose it from the crate it was.\nThe problem is that just this is hell.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"1a67f12975bcb909fb6a3d969813bdadfd86e087","unresolved":false,"context_lines":[{"line_number":19,"context_line":"   async (like the gRPC server)"},{"line_number":20,"context_line":" - We had to write our own custom glue code (SyncReadIntoAsyncRead)"},{"line_number":21,"context_line":"   to convert a sync io::Read into a tokio::io::AsyncRead, which also"},{"line_number":22,"context_line":"   seemed to be hard to upstream."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This now makes the BlobService trait async (via the async_trait macro,"},{"line_number":25,"context_line":"like we already do in various gRPC parts), and replaces the sync readers"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"12ef7b5c_969a1e62","line":22,"in_reply_to":"f96afd8a_e6e4ea6b","updated":"2023-09-15 21:00:44.000000000","message":"I added something around these lines, but skipped the hell part.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"3e9a07e1c1951adc9866769f9df7a747a9a88aea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8a6205d6_e742326a","updated":"2023-09-15 09:25:10.000000000","message":"\u003e We previously kept the trait of a BlobService sync.\n\nYeah, mixing synchronous I/O and asynchronous I/O just isn\u0027t really workable.\n\nYou can do it on a small scale, if you\u0027re super super careful to never do anything that blocks from an async context, but it\u0027s fragile.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3edfcffa_bac691a8","updated":"2023-09-15 20:11:00.000000000","message":"Looks good to me modulo testing.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"b8c71544b08888932d619b42a245925543c46091","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"1d41246b_e8e4a099","updated":"2023-09-15 21:06:45.000000000","message":"Tested import/fuse locally, works for me","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"}],"tvix/cli/src/tvix_store_io.rs":[{"author":{"_account_id":1000066,"name":"Adam Joseph","display_name":"amjoseph","email":"adam@westernsemico.com","username":"amjoseph"},"change_message_id":"3e9a07e1c1951adc9866769f9df7a747a9a88aea","unresolved":true,"context_lines":[{"line_number":289,"context_line":"/// [PathInfoService]."},{"line_number":290,"context_line":"#[instrument(skip(blob_service, directory_service, path_info_service), ret, err)]"},{"line_number":291,"context_line":"async fn import_path_with_pathinfo("},{"line_number":292,"context_line":"    blob_service: Arc\u003cdyn BlobService\u003e,"},{"line_number":293,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":294,"context_line":"    path_info_service: Arc\u003cdyn PathInfoService\u003e,"},{"line_number":295,"context_line":"    path: \u0026std::path::Path,"},{"line_number":296,"context_line":") -\u003e Result\u003cPathInfo, io::Error\u003e {"},{"line_number":297,"context_line":"    // Call [import::ingest_path], which will walk over the given path and return a root_node."},{"line_number":298,"context_line":"    let root_node \u003d import::ingest_path(blob_service.clone(), directory_service.clone(), path)"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"f2a7e3da_256e5ed7","line":295,"range":{"start_line":292,"start_character":0,"end_line":295,"end_character":0},"updated":"2023-09-15 09:25:10.000000000","message":"Could you use generics instead of `dyn` here?","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d43138ebbf3bc0d139278f3b0b2e34f0650f8fd0","unresolved":false,"context_lines":[{"line_number":289,"context_line":"/// [PathInfoService]."},{"line_number":290,"context_line":"#[instrument(skip(blob_service, directory_service, path_info_service), ret, err)]"},{"line_number":291,"context_line":"async fn import_path_with_pathinfo("},{"line_number":292,"context_line":"    blob_service: Arc\u003cdyn BlobService\u003e,"},{"line_number":293,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":294,"context_line":"    path_info_service: Arc\u003cdyn PathInfoService\u003e,"},{"line_number":295,"context_line":"    path: \u0026std::path::Path,"},{"line_number":296,"context_line":") -\u003e Result\u003cPathInfo, io::Error\u003e {"},{"line_number":297,"context_line":"    // Call [import::ingest_path], which will walk over the given path and return a root_node."},{"line_number":298,"context_line":"    let root_node \u003d import::ingest_path(blob_service.clone(), directory_service.clone(), path)"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"1840d488_e31bcef7","line":295,"range":{"start_line":292,"start_character":0,"end_line":295,"end_character":0},"in_reply_to":"9d818b9f_86026fd4","updated":"2023-09-17 19:37:26.000000000","message":"Marking this as resolved.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"af48519974fa365b8bd7f3d7236f0ee58662c1c1","unresolved":true,"context_lines":[{"line_number":289,"context_line":"/// [PathInfoService]."},{"line_number":290,"context_line":"#[instrument(skip(blob_service, directory_service, path_info_service), ret, err)]"},{"line_number":291,"context_line":"async fn import_path_with_pathinfo("},{"line_number":292,"context_line":"    blob_service: Arc\u003cdyn BlobService\u003e,"},{"line_number":293,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":294,"context_line":"    path_info_service: Arc\u003cdyn PathInfoService\u003e,"},{"line_number":295,"context_line":"    path: \u0026std::path::Path,"},{"line_number":296,"context_line":") -\u003e Result\u003cPathInfo, io::Error\u003e {"},{"line_number":297,"context_line":"    // Call [import::ingest_path], which will walk over the given path and return a root_node."},{"line_number":298,"context_line":"    let root_node \u003d import::ingest_path(blob_service.clone(), directory_service.clone(), path)"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"9d818b9f_86026fd4","line":295,"range":{"start_line":292,"start_character":0,"end_line":295,"end_character":0},"in_reply_to":"ab21f779_b5f7804b","updated":"2023-09-15 09:46:48.000000000","message":"this will essentially be a different concrete type depending on how you layer your stores together, so all we know is that it speaks this interface.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"0bb4324458724276f91d16aca7e4addb179fbb1e","unresolved":true,"context_lines":[{"line_number":289,"context_line":"/// [PathInfoService]."},{"line_number":290,"context_line":"#[instrument(skip(blob_service, directory_service, path_info_service), ret, err)]"},{"line_number":291,"context_line":"async fn import_path_with_pathinfo("},{"line_number":292,"context_line":"    blob_service: Arc\u003cdyn BlobService\u003e,"},{"line_number":293,"context_line":"    directory_service: Arc\u003cdyn DirectoryService\u003e,"},{"line_number":294,"context_line":"    path_info_service: Arc\u003cdyn PathInfoService\u003e,"},{"line_number":295,"context_line":"    path: \u0026std::path::Path,"},{"line_number":296,"context_line":") -\u003e Result\u003cPathInfo, io::Error\u003e {"},{"line_number":297,"context_line":"    // Call [import::ingest_path], which will walk over the given path and return a root_node."},{"line_number":298,"context_line":"    let root_node \u003d import::ingest_path(blob_service.clone(), directory_service.clone(), path)"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"ab21f779_b5f7804b","line":295,"range":{"start_line":292,"start_character":0,"end_line":295,"end_character":0},"in_reply_to":"f2a7e3da_256e5ed7","updated":"2023-09-15 09:46:10.000000000","message":"no, because the exact type of store is configurable as a cmdline parameter. at least it is for tvix-store CLI right now, and I assume it\u0027ll be for tvix-cli too.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"}],"tvix/store/src/bin/tvix-store.rs":[{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"9b2130c03135ac8e1856861e3de643eca0cacdfe","unresolved":true,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"                        // Ask the PathInfoService for the NAR size and sha256"},{"line_number":212,"context_line":"                        let (nar_size, nar_sha256) \u003d path_info_service.calculate_nar(\u0026root_node)?;"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"                        // TODO: make a path_to_name helper function?"},{"line_number":215,"context_line":"                        let name \u003d path"},{"line_number":216,"context_line":"                            .file_name()"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"339f0720_524f0104","line":213,"updated":"2023-09-15 11:26:41.000000000","message":"I\u0027m getting a panic when doing a `tvix-store import` now. Backtrace:\n\n```\n$ RUST_BACKTRACE\u003d1 ../target/debug/tvix-store import ../docs\nthread \u0027tokio-runtime-worker\u0027 panicked at \u0027Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.\u0027, store/src/pathinfoservice/grpc.rs:155:14\nstack backtrace:\n   0: rust_begin_unwind\n   1: core::panicking::panic_fmt\n   2: tokio::runtime::context::runtime::enter_runtime\n             at /home/cbrewster/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/runtime.rs:68:5\n   3: tokio::runtime::handle::Handle::block_on\n             at /home/cbrewster/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/handle.rs:309:9\n   4: \u003ctvix_store::pathinfoservice::grpc::GRPCPathInfoService as tvix_store::pathinfoservice::PathInfoService\u003e::calculate_nar\n             at ./src/pathinfoservice/grpc.rs:153:20\n   5: tvix_store::main::{{closure}}::{{closure}}::{{closure}}\n             at ./src/bin/tvix-store.rs:212:54\n...\n```\n\nI think the `calculate_nar` call needs to be in a `spawn_blocking` still since it calls `block_on` in the gRPC implementation. Will be a non-issue once the path_info service is asyncified.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8c23dce61d3dc564611261a09b8933ef2dc8fdd9","unresolved":true,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"                        // Ask the PathInfoService for the NAR size and sha256"},{"line_number":212,"context_line":"                        let (nar_size, nar_sha256) \u003d path_info_service.calculate_nar(\u0026root_node)?;"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"                        // TODO: make a path_to_name helper function?"},{"line_number":215,"context_line":"                        let name \u003d path"},{"line_number":216,"context_line":"                            .file_name()"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"6672f8a1_0d06c352","line":213,"in_reply_to":"339f0720_524f0104","updated":"2023-09-15 11:31:40.000000000","message":"Yes, I need to do the same there as I did in the tvix-cli binary. new revision incoming a bit later today.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"fd72157a2a0ea824b240bd265d81ed3b758ee140","unresolved":true,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"                        // Ask the PathInfoService for the NAR size and sha256"},{"line_number":212,"context_line":"                        let (nar_size, nar_sha256) \u003d path_info_service.calculate_nar(\u0026root_node)?;"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"                        // TODO: make a path_to_name helper function?"},{"line_number":215,"context_line":"                        let name \u003d path"},{"line_number":216,"context_line":"                            .file_name()"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"c3185e86_a5b861af","line":213,"in_reply_to":"6672f8a1_0d06c352","updated":"2023-09-15 12:05:10.000000000","message":"...and some regression tests for the binary, this has happened too often;-)","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"a9ca6400b77f0bab08e1b4e93ddec15f66ffd9fc","unresolved":false,"context_lines":[{"line_number":210,"context_line":""},{"line_number":211,"context_line":"                        // Ask the PathInfoService for the NAR size and sha256"},{"line_number":212,"context_line":"                        let (nar_size, nar_sha256) \u003d path_info_service.calculate_nar(\u0026root_node)?;"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"                        // TODO: make a path_to_name helper function?"},{"line_number":215,"context_line":"                        let name \u003d path"},{"line_number":216,"context_line":"                            .file_name()"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"39d2ed39_1745804c","line":213,"in_reply_to":"c3185e86_a5b861af","updated":"2023-09-15 12:57:26.000000000","message":"Fixed in a later revision, integration test added to cl/9338.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"2035f3db0966394964a05be9da9c562aac22ac95","unresolved":true,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"                        // put into [PathInfoService], and return the PathInfo that we get back"},{"line_number":251,"context_line":"                        // from there (it might contain additional signatures)."},{"line_number":252,"context_line":"                        let path_info \u003d path_info_service.put(path_info)?;"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"                        let node \u003d path_info.node.unwrap().node.unwrap();"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":7,"id":"9a61bb33_c6bed215","line":252,"updated":"2023-09-15 18:40:13.000000000","message":"Panic has moved here, this one needs `spawn_blocking` too","commit_id":"81dbd7659f43e795bb0a4758e5fb3fc306b8c5f1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"1a67f12975bcb909fb6a3d969813bdadfd86e087","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"                        // put into [PathInfoService], and return the PathInfo that we get back"},{"line_number":251,"context_line":"                        // from there (it might contain additional signatures)."},{"line_number":252,"context_line":"                        let path_info \u003d path_info_service.put(path_info)?;"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"                        let node \u003d path_info.node.unwrap().node.unwrap();"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":7,"id":"5546d62b_9d5373de","line":252,"in_reply_to":"1526c095_509e1dc9","updated":"2023-09-15 21:00:44.000000000","message":"Done","commit_id":"81dbd7659f43e795bb0a4758e5fb3fc306b8c5f1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"63d1f6fa52001d039917894f9c57eb253b779cd0","unresolved":true,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"                        // put into [PathInfoService], and return the PathInfo that we get back"},{"line_number":251,"context_line":"                        // from there (it might contain additional signatures)."},{"line_number":252,"context_line":"                        let path_info \u003d path_info_service.put(path_info)?;"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"                        let node \u003d path_info.node.unwrap().node.unwrap();"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":7,"id":"1526c095_509e1dc9","line":252,"in_reply_to":"9a61bb33_c6bed215","updated":"2023-09-15 18:58:07.000000000","message":"Huuuh. thanks. will fix","commit_id":"81dbd7659f43e795bb0a4758e5fb3fc306b8c5f1"}],"tvix/store/src/blobservice/dumb_seeker.rs":[{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"9b2130c03135ac8e1856861e3de643eca0cacdfe","unresolved":true,"context_lines":[{"line_number":158,"context_line":"                let mut bytes_to_skip \u003d self.bytes_to_skip - bytes_read;"},{"line_number":159,"context_line":"                let pos \u003d self.pos;"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"                self.project().bytes_to_skip \u003d \u0026mut bytes_to_skip;"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"571dd609_67b84ba9","line":161,"updated":"2023-09-15 11:26:41.000000000","message":"I didn\u0027t know this was supported, I usually see this like:\n```\n*self.project().bytes_to_skip \u003d bytes_to_skip;\n```","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"a9ca6400b77f0bab08e1b4e93ddec15f66ffd9fc","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                let mut bytes_to_skip \u003d self.bytes_to_skip - bytes_read;"},{"line_number":159,"context_line":"                let pos \u003d self.pos;"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"                self.project().bytes_to_skip \u003d \u0026mut bytes_to_skip;"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"10585c8d_33b2ac82","line":161,"in_reply_to":"571dd609_67b84ba9","updated":"2023-09-15 12:57:26.000000000","message":"Yes, this looks a lit less convoluted. I also updated a similar instance of this in L117.","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":true,"context_lines":[{"line_number":8,"context_line":"pin_project! {"},{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"2d1bffcd_ea2f0092","line":11,"updated":"2023-09-15 20:11:00.000000000","message":"Explain why we are pinning only `r` and not `pos`, `bytes_to_skip` even though this is safe.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"21b2625894670b53c89509163e9c2468f96b1349","unresolved":true,"context_lines":[{"line_number":8,"context_line":"pin_project! {"},{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"e176a309_aa5e776a","line":11,"in_reply_to":"2d1bffcd_ea2f0092","updated":"2023-09-15 21:09:17.000000000","message":"Let\u0027s explain it.\n\n`NaiveSeeker` itself is pinned somewhere and not allowed to move, though\nits fields could potentially be pinned or unpinned according to https://doc.rust-lang.org/std/pin/#pinning-is-not-structural-for-field\n\nIn this situation, we want to ensure that neither `pos` neither `bytes_to_skip` are pinned, and we will want to ensure we never write `pin!(pos)` or similar to ensure this invariant to be true.\n\nFor `r`, it is more complicated, we definitely may be pinning it in some instances, so let\u0027s mark it as pinned all the time.\n\nAnyway, we need to use it as pinned, so it all works well for us.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d43138ebbf3bc0d139278f3b0b2e34f0650f8fd0","unresolved":true,"context_lines":[{"line_number":8,"context_line":"pin_project! {"},{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"f18f51f7_0b9006a9","line":11,"in_reply_to":"a139fcc1_0d335b1e","updated":"2023-09-17 19:37:26.000000000","message":"We need to update the comment w.r.t. https://cl.tvl.fyi/c/depot/+/9329/comment/21de931b_5b672f4b/.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"a694ab66008684f01f9835af36241490f67c180c","unresolved":true,"context_lines":[{"line_number":8,"context_line":"pin_project! {"},{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"a139fcc1_0d335b1e","line":11,"in_reply_to":"e176a309_aa5e776a","updated":"2023-09-16 14:59:21.000000000","message":"Another attempt:\n\n`NaiveSeeker` is itself pinned by callers, and we do not need to concern ourselves regarding that.\n\nThough, its fields as per https://doc.rust-lang.org/std/pin/#pinning-is-not-structural-for-field can be pinned or unpinned.\n\nSo we need to go over each field and choose our policy carefully.\n\nThe obvious cases are the bookkeeping integers we keep in the structure, those are private and not shared to anyone, we never build a `Pin\u003c\u0026mut X\u003e` out of them at any point, therefore, we can safely never mark them as pinned. Of course, it is expected that no developer here attempt to `pin!(pos)` to pin them because it makes no sense. If they have to become pinned, they should be marked `#[pin]` and we need to discuss it.\n\nSo the bookkeeping integers are in the right state with respect to their pinning status. The projection should offer direct access.\n\nOn the `r` field, i.e. a `BufReader\u003cR\u003e`, given that https://docs.rs/tokio/latest/tokio/io/struct.BufReader.html#impl-Unpin-for-BufReader%3CR%3E is available, even a `Pin\u003c\u0026mut BufReader\u003cR\u003e\u003e` can be safely moved.\n\nThe only care we should have regards the internal reader itself, i.e. the `R` instance, see that Tokio decided to `#[pin]` it too: https://docs.rs/tokio/latest/src/tokio/io/util/buf_reader.rs.html#29\n\nIn general, there\u0027s no `Unpin` instance for `R: tokio::io::AsyncRead` (see https://docs.rs/tokio/latest/tokio/io/trait.AsyncRead.html).\n\nTherefore, we could keep it unpinned and pin it in every call site whenever we need to call `poll_*` which can be confusing to the non-expert developer and we have a fair share amount of situations where the `BufReader` instance is naked, i.e. in its `\u0026mut BufReader\u003cR\u003e` form, this is annoying because it could lead to expose the naked `R` internal instance somehow and would produce a risk of making it move unexpectedly.\n\nWe choose the path of the least resistance as we have no reason to have access to the raw `BufReader\u003cR\u003e` instance, we just `#[pin]` it too and enjoy its `poll_*` safe APIs and push the unpinning concerns to the internal implementations themselves, which studied the question longer than us.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"f357184335815b19806b19b94d065eba6cce0bce","unresolved":false,"context_lines":[{"line_number":8,"context_line":"pin_project! {"},{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"1d2f1dfc_5fe1cc1a","line":11,"in_reply_to":"f18f51f7_0b9006a9","updated":"2023-09-18 10:23:47.000000000","message":"The comment is still up to date, added to a new CL revision.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":true,"context_lines":[{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"},{"line_number":15,"context_line":"        pos: u64,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"776e46b0_4604b6de","line":12,"updated":"2023-09-15 20:11:00.000000000","message":"I prefer `NaiveSeeker` than the pejorative term \"dumb\".","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"1a67f12975bcb909fb6a3d969813bdadfd86e087","unresolved":false,"context_lines":[{"line_number":9,"context_line":"    /// This implements [tokio::io::AsyncSeek] for and [tokio::io::AsyncRead] by"},{"line_number":10,"context_line":"    /// simply skipping over some bytes, keeping track of the position."},{"line_number":11,"context_line":"    /// It fails whenever you try to seek backwards."},{"line_number":12,"context_line":"    pub struct DumbSeeker\u003cR: tokio::io::AsyncRead\u003e {"},{"line_number":13,"context_line":"        #[pin]"},{"line_number":14,"context_line":"        r: tokio::io::BufReader\u003cR\u003e,"},{"line_number":15,"context_line":"        pos: u64,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"3f10e387_a070de30","line":12,"in_reply_to":"776e46b0_4604b6de","updated":"2023-09-15 21:00:44.000000000","message":"Done","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"a36ad24beb6aee3edcd80591b10b044751c4d533","unresolved":true,"context_lines":[{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"},{"line_number":165,"context_line":"                } else {"},{"line_number":166,"context_line":"                    Poll::Pending"},{"line_number":167,"context_line":"                }"},{"line_number":168,"context_line":"            }"},{"line_number":169,"context_line":"            Poll::Pending \u003d\u003e Poll::Pending,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"21de931b_5b672f4b","line":166,"updated":"2023-09-17 12:50:18.000000000","message":"I was having some trouble where `seek` that requires multiple reads would get stuck. I added some logging and `poll_complete` was polled once but never polled again.\n\nI think the [docs](https://doc.rust-lang.org/stable/std/task/enum.Poll.html#variant.Pending) about `Poll::Pending` are relevant:\n\n\u003e When a function returns Pending, the function must also ensure that the current task is scheduled to be awoken when progress can be made.\n\nWhat happens here is we return `Poll::Pending` but we don\u0027t do anything to ensure that this task is woken up again.\n\nI did some testing and one solution to this is to put the `poll_read` in a loop, and in this case trigger another `poll_read` instead of returning `Poll::Pending`. Likely the call to `poll_read` will end up returning `Poll::Pending`, but this time the I/O operation will be registered with the waker so the future will get polled again and things will work as expected","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"89a9eff19ab6f0f2489b4955d1436f4407c698d7","unresolved":true,"context_lines":[{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"},{"line_number":165,"context_line":"                } else {"},{"line_number":166,"context_line":"                    Poll::Pending"},{"line_number":167,"context_line":"                }"},{"line_number":168,"context_line":"            }"},{"line_number":169,"context_line":"            Poll::Pending \u003d\u003e Poll::Pending,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"4eb2479b_2640a719","line":166,"in_reply_to":"21de931b_5b672f4b","updated":"2023-09-17 12:58:36.000000000","message":"Here\u0027s a test case to demonstrate the issue:\n```\n    #[tokio::test]\n    async fn seek() {\n        let buf \u003d vec![0u8; 4096];\n        let reader \u003d Cursor::new(\u0026buf);\n        let mut seeker \u003d NaiveSeeker::new(reader);\n        seeker.seek(SeekFrom::Start(4000)).await.unwrap();\n    }\n```\n\nThis seek will require multiple `poll_reads` since we use a 1024 byte internal buffer when doing the seek.\n\nAs-is this test will hang indefinitely.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"3d0aa3896ccc84f9e9c2b61fba581358732c9893","unresolved":true,"context_lines":[{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"},{"line_number":165,"context_line":"                } else {"},{"line_number":166,"context_line":"                    Poll::Pending"},{"line_number":167,"context_line":"                }"},{"line_number":168,"context_line":"            }"},{"line_number":169,"context_line":"            Poll::Pending \u003d\u003e Poll::Pending,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"a5739bcd_5234a3da","line":166,"in_reply_to":"4eb2479b_2640a719","updated":"2023-09-17 13:19:54.000000000","message":"Much agreed, the solution suggested is not that bad, either case, we have to kind of do our bookkeeping ourselves and call `cx` or pass it to someone else who knows how to call it, e.g. internal poll_read impl.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"14eca748a08648fe066e18d92df734b9d2e2c73f","unresolved":true,"context_lines":[{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"},{"line_number":165,"context_line":"                } else {"},{"line_number":166,"context_line":"                    Poll::Pending"},{"line_number":167,"context_line":"                }"},{"line_number":168,"context_line":"            }"},{"line_number":169,"context_line":"            Poll::Pending \u003d\u003e Poll::Pending,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"cacd6c9d_578049ab","line":166,"in_reply_to":"a5739bcd_5234a3da","updated":"2023-09-17 15:03:39.000000000","message":"I included that test, so we see if it times out indefinitely. Thanks!\n\nRight now the code you sent me earlier does crash, looks like because it doesn\u0027t stop skipping where it should be. I need to take another look at this later.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d43138ebbf3bc0d139278f3b0b2e34f0650f8fd0","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                if bytes_to_skip \u003d\u003d 0 {"},{"line_number":164,"context_line":"                    Poll::Ready(Ok(pos))"},{"line_number":165,"context_line":"                } else {"},{"line_number":166,"context_line":"                    Poll::Pending"},{"line_number":167,"context_line":"                }"},{"line_number":168,"context_line":"            }"},{"line_number":169,"context_line":"            Poll::Pending \u003d\u003e Poll::Pending,"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"2be9e0ff_d26462d9","line":166,"in_reply_to":"cacd6c9d_578049ab","updated":"2023-09-17 19:37:26.000000000","message":"The fixes from cl/9356 are now squashed in, so this should be fixed. Let\u0027s continue the docs discussion in https://cl.tvl.fyi/c/depot/+/9329/comment/2d1bffcd_ea2f0092/, marking this as resolved.","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"}],"tvix/store/src/blobservice/grpc.rs":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":true,"context_lines":[{"line_number":390,"context_line":"            // wait for the socket to be created"},{"line_number":391,"context_line":"            {"},{"line_number":392,"context_line":"                let mut socket_created \u003d false;"},{"line_number":393,"context_line":"                for _try in 1..20 {"},{"line_number":394,"context_line":"                    if path.exists() {"},{"line_number":395,"context_line":"                        socket_created \u003d true;"},{"line_number":396,"context_line":"                        break;"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"faa6577f_846ae6c4","line":393,"updated":"2023-09-15 20:11:00.000000000","message":"this is absolutely random, isn\u0027t it?\nplease invest in proper exp backoff or add a TODO to do it \"urgently\"\n\neven if this is for a test, you should avoid this because if your testing machine is under high load, some stuff may happen and you won\u0027t be happy\n\noverall, you should define a global target timeout for the test","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"1a67f12975bcb909fb6a3d969813bdadfd86e087","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            // wait for the socket to be created"},{"line_number":391,"context_line":"            {"},{"line_number":392,"context_line":"                let mut socket_created \u003d false;"},{"line_number":393,"context_line":"                for _try in 1..20 {"},{"line_number":394,"context_line":"                    if path.exists() {"},{"line_number":395,"context_line":"                        socket_created \u003d true;"},{"line_number":396,"context_line":"                        break;"}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"7ae2a0b0_f1eaa340","line":393,"in_reply_to":"faa6577f_846ae6c4","updated":"2023-09-15 21:00:44.000000000","message":"Done","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"13772cfc5b20fc6e522f4dc291c0e5a9467e2a65","unresolved":true,"context_lines":[{"line_number":273,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":274,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":275,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":276,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":277,"context_line":"    }"},{"line_number":278,"context_line":"}"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"72c64e41_5881f42b","line":276,"updated":"2023-09-15 21:27:01.000000000","message":"I think we need to think more about this.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"54e1953d44a9528ea50e2bd77f6f39295be4751f","unresolved":true,"context_lines":[{"line_number":273,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":274,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":275,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":276,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":277,"context_line":"    }"},{"line_number":278,"context_line":"}"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"517084f6_3cfedca8","line":276,"in_reply_to":"0636b4e2_1c55676b","updated":"2023-09-17 15:01:40.000000000","message":"Did you do any digging up of tonic docs so far?","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"f357184335815b19806b19b94d065eba6cce0bce","unresolved":false,"context_lines":[{"line_number":273,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":274,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":275,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":276,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":277,"context_line":"    }"},{"line_number":278,"context_line":"}"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"53d75fdb_0e8bce44","line":276,"in_reply_to":"517084f6_3cfedca8","updated":"2023-09-18 10:23:47.000000000","message":"Discussed this with @raitobezarius. This might cause individual channels inside a gRPC connection to not be shut down properly, but we both agreed to keep this for a future todo and logged a comment.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"3d0aa3896ccc84f9e9c2b61fba581358732c9893","unresolved":true,"context_lines":[{"line_number":273,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":274,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":275,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":276,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":277,"context_line":"    }"},{"line_number":278,"context_line":"}"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"0636b4e2_1c55676b","line":276,"in_reply_to":"72c64e41_5881f42b","updated":"2023-09-17 13:19:54.000000000","message":"This is the only one that is hard per se.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"}],"tvix/store/src/blobservice/memory.rs":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":109,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":110,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":111,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":112,"context_line":"    }"},{"line_number":113,"context_line":"}"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"dc604328_e6372d65","line":111,"updated":"2023-09-15 20:11:00.000000000","message":"let\u0027s explain why we don\u0027t need to poll shutdown, even if it\u0027s obvious to us","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"984326b6aaf3f88c4ce64598f4eed6f494ed653c","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":109,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":110,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":111,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":112,"context_line":"    }"},{"line_number":113,"context_line":"}"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"506db26c_efcf7818","line":111,"in_reply_to":"54b4c8bd_dacc1278","updated":"2023-09-15 21:10:17.000000000","message":"for sled: this is the same, closing a sled connection is closing a file handle, right?\n\nfor grpc: unknown?","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"1a67f12975bcb909fb6a3d969813bdadfd86e087","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":109,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":110,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":111,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":112,"context_line":"    }"},{"line_number":113,"context_line":"}"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"54b4c8bd_dacc1278","line":111,"in_reply_to":"dc604328_e6372d65","updated":"2023-09-15 21:00:44.000000000","message":"Done","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"13772cfc5b20fc6e522f4dc291c0e5a9467e2a65","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":110,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":111,"context_line":"        // shutdown is \"instantaneous\""},{"line_number":112,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":113,"context_line":"    }"},{"line_number":114,"context_line":"}"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"ddcd1f07_b26512e5","line":112,"updated":"2023-09-15 21:27:01.000000000","message":"I think we need to think more about this.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"54e1953d44a9528ea50e2bd77f6f39295be4751f","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":110,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":111,"context_line":"        // shutdown is \"instantaneous\""},{"line_number":112,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":113,"context_line":"    }"},{"line_number":114,"context_line":"}"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"7bce1932_5febde6b","line":112,"in_reply_to":"5e8beb55_88a3e5aa","updated":"2023-09-17 15:01:40.000000000","message":"Done","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"3d0aa3896ccc84f9e9c2b61fba581358732c9893","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":110,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":111,"context_line":"        // shutdown is \"instantaneous\""},{"line_number":112,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":113,"context_line":"    }"},{"line_number":114,"context_line":"}"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"5e8beb55_88a3e5aa","line":112,"in_reply_to":"ddcd1f07_b26512e5","updated":"2023-09-17 13:19:54.000000000","message":"Memory is trivial: interaction with memory is synchronous and instant, so you never poll a shutdown.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"}],"tvix/store/src/blobservice/mod.rs":[{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"9b2130c03135ac8e1856861e3de643eca0cacdfe","unresolved":true,"context_lines":[{"line_number":26,"context_line":"pub trait BlobService: Send + Sync {"},{"line_number":27,"context_line":"    /// Create a new instance by passing in a connection URL."},{"line_number":28,"context_line":"    /// TODO: check if we want to make this async, instead of lazily connecting"},{"line_number":29,"context_line":"    fn from_url(url: \u0026url::Url) -\u003e Result\u003cSelf, Error\u003e"},{"line_number":30,"context_line":"    where"},{"line_number":31,"context_line":"        Self: Sized;"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"3938c4f6_b21ea984","line":29,"updated":"2023-09-15 11:26:41.000000000","message":"Unrelated to this cl, but any reason to include this in the trait? I usually try to keep \"constructors\" out of traits since different implementations might be constructed differently. (async vs sync depending on the impl is a potential example of this)","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"ff96ebea1bd9e02f4ec01f3ed2176d49354f1539","unresolved":true,"context_lines":[{"line_number":26,"context_line":"pub trait BlobService: Send + Sync {"},{"line_number":27,"context_line":"    /// Create a new instance by passing in a connection URL."},{"line_number":28,"context_line":"    /// TODO: check if we want to make this async, instead of lazily connecting"},{"line_number":29,"context_line":"    fn from_url(url: \u0026url::Url) -\u003e Result\u003cSelf, Error\u003e"},{"line_number":30,"context_line":"    where"},{"line_number":31,"context_line":"        Self: Sized;"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"68d891a9_bb3394b6","line":29,"in_reply_to":"3938c4f6_b21ea984","updated":"2023-09-15 12:07:45.000000000","message":"The idea was to make different store protocols pluggable by their URI, but something needs to collect them all together anyways. Happy to shuffle things around though, let\u0027s do it in a followup?","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"63faf94d6be3ea1bacb41a7f545d8c839efc93ec","unresolved":false,"context_lines":[{"line_number":26,"context_line":"pub trait BlobService: Send + Sync {"},{"line_number":27,"context_line":"    /// Create a new instance by passing in a connection URL."},{"line_number":28,"context_line":"    /// TODO: check if we want to make this async, instead of lazily connecting"},{"line_number":29,"context_line":"    fn from_url(url: \u0026url::Url) -\u003e Result\u003cSelf, Error\u003e"},{"line_number":30,"context_line":"    where"},{"line_number":31,"context_line":"        Self: Sized;"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"6bbfd612_6496b699","line":29,"in_reply_to":"68d891a9_bb3394b6","updated":"2023-09-15 12:43:17.000000000","message":"Sounds good!","commit_id":"75aeda5f1789cf84f71e4a085ad0d3863565ccf9"}],"tvix/store/src/blobservice/sled.rs":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"aa897b09895a3e23e68b7c6da162f0f39fbd3ea0","unresolved":true,"context_lines":[{"line_number":137,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":138,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":139,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":140,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":141,"context_line":"    }"},{"line_number":142,"context_line":"}"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"9547ad99_2e27bab9","line":140,"updated":"2023-09-15 20:11:00.000000000","message":"explain why this is obvious that we don\u0027t need to poll for shutdowns","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"1a67f12975bcb909fb6a3d969813bdadfd86e087","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        self: std::pin::Pin\u003c\u0026mut Self\u003e,"},{"line_number":138,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":139,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":140,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":141,"context_line":"    }"},{"line_number":142,"context_line":"}"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":8,"id":"fb1c2581_dca7eb92","line":140,"in_reply_to":"9547ad99_2e27bab9","updated":"2023-09-15 21:00:44.000000000","message":"Done","commit_id":"0f234aac3afd912eca9fb109d090470a517368e1"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"13772cfc5b20fc6e522f4dc291c0e5a9467e2a65","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":139,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":140,"context_line":"        // shutdown is \"instantaneous\""},{"line_number":141,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":142,"context_line":"    }"},{"line_number":143,"context_line":"}"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"85d02294_64d3e665","line":141,"updated":"2023-09-15 21:27:01.000000000","message":"I think we need to think more about this.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"54e1953d44a9528ea50e2bd77f6f39295be4751f","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":139,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":140,"context_line":"        // shutdown is \"instantaneous\""},{"line_number":141,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":142,"context_line":"    }"},{"line_number":143,"context_line":"}"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"bfbfcf90_8eb7d5bf","line":141,"in_reply_to":"66e9cff9_d4b593d0","updated":"2023-09-17 15:01:40.000000000","message":"Right now, a blob in sled is just a large value in a KV, and BlobWriter keeps a Vec\u003cu8\u003e until you call BlobWriter::close.","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"3d0aa3896ccc84f9e9c2b61fba581358732c9893","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        _cx: \u0026mut std::task::Context\u003c\u0027_\u003e,"},{"line_number":139,"context_line":"    ) -\u003e std::task::Poll\u003cResult\u003c(), io::Error\u003e\u003e {"},{"line_number":140,"context_line":"        // shutdown is \"instantaneous\""},{"line_number":141,"context_line":"        Poll::Ready(Ok(()))"},{"line_number":142,"context_line":"    }"},{"line_number":143,"context_line":"}"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":10,"id":"66e9cff9_d4b593d0","line":141,"in_reply_to":"85d02294_64d3e665","updated":"2023-09-17 13:19:54.000000000","message":"AFAIK, sled is also trivial: interaction with file handles is synchronous and instant in this scenario, we are not using an async API for sled, are we?\nAnd I am not so sure there\u0027s async close? Close is always sync?","commit_id":"8a62f05cfaefa7158284ab2f2f51c6adc77b9a01"}]}