Concurrent access and database vs file_name

[maxmcd]

Hey @danthegoodman1, I've been enjoying using your library.

self.database and self.lease are currently held on the GrpcVfs struct, preventing concurrent access to different databases.

Do you know if there is a good path to resolution for this? Maybe parsing file path to construct the database each time? Or changing the proto semantics to do lease-per-file-path instead of per db?

Happy to help implement something here.

[danthegoodman1]

Hello!

I don't recall exactly why I made that decision, but I think it might have either been:

1. It was easier at the time to get something working
2. Or atomic batch writes did not support cross-db operations because they need the WAL and shared mem, all things I was specifically avoiding

I don't recall exactly, maybe worth investigating to see if there are any limitations, as having no WAL, no shared mem, and atomic batch writes is the most performant version of gRPSQLite by a lot, so I wouldn't want to compromise that.

A short-term solution is just to have multiple sqlite connections open. But I am open to modifications it if it doesn't sacrifice performance.

[maxmcd]

Ok, just to preface this I'm still figuring this out, and not super confident, but:

I think sqlite is multi-threaded? And this struct can be accessed in multiple threads? And that you could open a process with this dylib and write to the db from multiple threads? I see that the vfs has optional .lock .unlock functions for file locking that are not implemented.

At the very least the specific case I ran into this bug is that within the same rust thread I opened two different databases, and I think the GrpcVfs was unable to support that as I switched between them.

I think you might need to track state per-file/db, and the structure could be something like this:

#[derive(Clone)]
struct FileState {
    pending_writes: Arc<Mutex<Vec<BatchWrite>>>,
    batch_open: Arc<AtomicBool>,
}

impl FileState {
    fn new() -> Self {
        Self {
            pending_writes: Arc::new(Mutex::new(Vec::new())),
            batch_open: Arc::new(AtomicBool::new(false)),
        }
    }
}

struct GrpcVfs {
    runtime: tokio::runtime::Runtime,
    capabilities: Capabilities,
    db: Arc<TokioMutex<Db>>,
    files: Arc<Mutex<HashMap<String, FileState>>>,
}

You would also need your caching to now be per db/filepath, etc.

Hopefully batch writes are supported regardless!

[danthegoodman1]

So if you need something per-file, the handle::GrpcVfsHandle is the right place to store that. It's created per file (per open() call), and handed around basically every method. I think it might have been the few methods (like device_characteristics) that don't pass the handle is why I made things global, since in theory you could choose to serve different capabilities per-file. But, if you add these to the handle, and assume capabilities as the process-level, then I think it might work?