Commit Graph

10 Commits

Author SHA1 Message Date
Siddharth Agarwal
4e9f826728 blobstore: restrict the types that can be stored
Summary:
Currently, any sort of `Bytes` can be stored in the blobstore. That
caused me to make several mistakes while writing the code to store bonsai
changesets, because I'd just end up storing the wrong set of `Bytes`.

Introduce stronger typing so that only types that explicitly implement
`BlobStorable` can be stored in the blobstore.

Currently, these sorts of blobs can be stored in the blob store:

* `ChangesetBlob` and `ContentBlob` in `mononoke-types` (these are Thrift-serialized structures)
* The envelope `RawNodeBlob` and `RawCSBlob` types in `blobrepo`, once converted to `EnvelopeBlob` instances
* `HgBlob`, which contains revlog data (manifests or files) exactly as serialized by Mercurial

Reviewed By: StanislavGlebik

Differential Revision: D7627290

fbshipit-source-id: d1bcbde8881e365dec99618556e7b054985bccf7
2018-04-18 19:53:32 -07:00
Simon Farnsworth
9e366eacd4 Introduce a LazyMemblob test blobstore
Summary:
The current Memblob store is eager; this is great for finding certain
classes of bugs (those that assume an ordering that is not guaranteed), but not
so good for exposing other classes of bugs (those that assume that the future
has done its work before it resolves).

Add a lazy variant that functions in the same way as Memblob, but that waits
until it's polled to return.

Reviewed By: StanislavGlebik

Differential Revision: D7033792

fbshipit-source-id: 4c2d8a8150d908bcb26347757f96f99e20d74fc2
2018-02-21 07:17:00 -08:00
Simon Farnsworth
28caf3f938 Remove Error, GetBlob and PutBlob from BlobStore
Summary:
Remove the last associated types from BlobStore - this means that
BlobStore now has an associated trait object type.

Reviewed By: jsgf

Differential Revision: D6425414

fbshipit-source-id: 7186dab9b56593dd1d70be732d4ad56d1e7b3c63
2017-12-11 07:05:53 -08:00
Jeremy Fitzhardinge
ac31713c84 rust: failure cleanup pass
Summary:
Don't use failure's bail!() and ensure!() macros.

Instead, failure_ext provides:
- bail_err!(err) - Converts its single parameter to the expected error and returns; ie `return Err(From::from(err));`
- bail_msg!(fmt, ...) - takes format string parameters and returns a `failure::err_msg()` error
- ensure_err!(), ensure_msg!() - corresponding changes

Also:
- remove all stray references to error-chain
- remove direct references to failure_derive (it's reexported via failure and failure_ext)
- replace uses of `Err(foo)?;` with `bail_err!()` (since `bail_err` unconditionally returns, but `Err(x)?` does not in principle, which can affect type inference)

Reviewed By: kulshrax

Differential Revision: D6507717

fbshipit-source-id: 635fb6f8c96d185b195dff171ea9c8db9e83af10
2017-12-07 14:10:17 -08:00
Jeremy Fitzhardinge
dc5e78c1c1 rust: mass convert scm/mononoke/... to use failure
Summary:
Convert scm/mononoke to use failure, and update common/rust crates it depends on as well.

What it looks like is a lot of deleted code...

General strategy:
- common/rust/failure_ext adds some things that are in git failure that aren't yet in crates.io (`bail!` and `ensure!`, `Result<T, Error>`)
- everything returns `Result<T, failure::Error>`
- crates with real error get an error type, with a derived Fail implementation
  - replicate error-chain by defining an `enum ErrorKind` where the fields match the declared errors in the error! macro
- crates with dummy error-chain (no local errors) lose it
- `.chain_err()` -> `.context()` or `.with_context()`

So far the only place I've needed to extract an error is in a unit test.
Having a single unified error type has simplified a lot of things, and removed a lot of error type parameters, error conversion, etc, etc.

Reviewed By: sid0

Differential Revision: D6446584

fbshipit-source-id: 744640ca2997d4a85513c4519017f2e2e78a73f5
2017-12-05 18:11:13 -08:00
Simon Farnsworth
16da012250 Remove ValueIn/ValueOut from the BlobStore generic arguments.
Summary:
BlobStore is entirely generic, and puts no limits on its
implementations. Remove ValueIn and ValueOut type parameters, and insist that
all blobs are Bytes (as per production setups)

Reviewed By: StanislavGlebik

Differential Revision: D6425413

fbshipit-source-id: 455e526d8baebd0d0f1906941648acca89be4881
2017-12-04 10:22:09 -08:00
Simon Farnsworth
c1ee28dbc7 Remove Key from the BlobStore generic arguments.
Summary:
BlobStore is entirely generic, and puts no limits on its
implementations. Remove the "Key" type parameter, and insist that all keys are
String (as per production setups)

Reviewed By: StanislavGlebik

Differential Revision: D6425412

fbshipit-source-id: 1f1229bf8e001bf780964e883c6beb071e9ef1d8
2017-12-04 10:22:09 -08:00
Stanislau Hlebik
83f2eb90a5 mononoke: remove retries
Summary:
Putting retries on this layer is not very good, because it requires every
client to add RetryingBlobstore.

Reviewed By: kulshrax

Differential Revision: D6298254

fbshipit-source-id: dbdce7fe141f9e1511322e74a1258d3819a68eb5
2017-11-13 03:21:31 -08:00
Lukas Piatkowski
9712530c0e blobstore: add RetryingBlobstore that retries failed put/get operations with delay
Reviewed By: StanislavGlebik

Differential Revision: D6203017

fbshipit-source-id: 277fa267e86d2cb5eede241bf80dd8d1c90a3b96
2017-10-31 20:53:08 -07:00
Siddharth Agarwal
0a0c594e03 blobstore: unify blobstore tests
Summary:
Similar to heads/bookmarks, share blobstore tests across
implementations.

The trait bounds got a bit tricky, but I think I managed to get the ones we
need and no more.

Reviewed By: StanislavGlebik

Differential Revision: D6082649

fbshipit-source-id: 3a80adc95d6c1059dab7ec89e5ec5724370cc268
2017-10-18 15:20:39 -07:00