Blob.{Read,Write}At

[anacrolix]

I was previously using ReadAt/WriteAt on the Blob struct. I was curious why you moved away from that API back to to a wrapper that maintains an offset. It seems like ReadAt/WriteAt are a much better fit for the underlying API in sqlite, and I'm having to emulate that behaviour by seeking on the Blob now anyway.

[zombiezen]

So the problem is in the specification of io.ReaderAt:

Clients of ReadAt can execute parallel ReadAt calls on the same input source.

This is emphatically not safe to do with *sqlite.Blob, even in the crawshaw implementation, so implementing those methods seemed like a footgun. I have thought about adding convenience functions that use the underlying API, but then the functions would be specific to *sqlite.Blob, so you can't use an interface like io.ReadWriteSeeker.

The Seek function is quite lightweight, since it only does bounds checks, so the only real cost is LOC in callers. For heavy Seek users, a custom wrapper at the caller seemed not unduly burdensome to me, so that's where I landed.

[anacrolix]

Also noting a lot of the machinery adding to work around this enormous. In fact to reimplement ReaderAt I have to lock the blob anyway, seek around and then reset the original position to maintain the other contracts (Read/Write/Seek etc.) that are artifically added on top. I think in the original implementation I just whacked a io SectionReader on the blob and io.Copied with that. That would also function as expected if Blob.ReaderAt did it's own synchronization (say I had 2 independent io.Copys running as described, they would be fine).

[zombiezen]

I'm hesitant to add a mutex in *sqlite.Blob because it forces overhead for all callers, even those that are operating in a non-concurrent access pattern. If you're trying to do concurrent access, you probably should have separate connections to the database.

Also noting a lot of the machinery adding to work around this enormous.

Can you help me understand the burden that you're facing? I'm imaging for reading (untested):

type blobReaderAt struct {
  mu sync.Mutex
  b *sqlite.Blob
}

func (r *blobReaderAt) ReadAt(p []byte, off int64) (n int, err error) {
  r.mu.Lock()
  defer r.mu.Unlock()
  if _, err := r.b.Seek(off, io.SeekStart); err != nil {
    return 0, err
  }
  return io.ReadFull(r.b, p)
}

This should be enough to connect to an io.SectionReader and run concurrent (but contended) reads within a transaction.

[anacrolix]

https://github.com/anacrolix/squirrel/blob/3a6aaa90d078d51fc1d8ad2588298a3d68a4eeaf/compat.go#L12 (I think locking is done externally for this one, and clearly blobWriteAt is neglecting to update offsets correctly). Perhaps I should just wrap Blob in a ReaderAt/WriterAt implementation like you provide and ignore the other methods on Blob?

[zombiezen]

Yeah, that seems right to me.

[anacrolix]

Thank you