refactor: add database crates

[ethowitz]

Overview

This PR breaks up syncstorage's and tokenserver's database functionality into a number of new crates:

• syncserver-db-common: Database-related code that is shared between syncstorage and tokenserver
  • Does not depend on any other database crates
• syncstorage-db-common: Database-related code that is shared between the MySQL and Spanner backends of syncstorage. Note that this crate is not used in any way by Tokenserver
  • Depends on syncserver-db-common
• syncstorage-db: Acts as an interface between the two database backends and the syncserver crate
  • Depends on:
    • syncstorage-mysql
    • syncstorage-spanner
    • syncserver-db-common
    • syncstorage-db-common
• syncstorage-mysql: The code that powers syncstorage's MySQL database backend
  • Depends on:
    • syncstorage-db-common
    • syncserver-db-common
• syncstoarge-spanner: The code the powers syncstorage's Spanner database backend
  • Depends on:
    • syncstorage-db-common
    • syncserver-db-common
• tokenserver-db: The code that powers tokenserver's MySQL database backend
  • Depends on:
    • syncserver-db-common

I'd recommend generating the docs using cargo doc --no-deps --workspace to view the public interfaces for each crate in the workspace. I tried to keep the public interfaces for each crate as simple as possible.

Crate Relationships

It might be useful to visualize how the new crates relate to one another (I generated this using cargo depgraph):

The idea is that syncserver should not need to know any details about the underlying database backend. syncstorage-db acts as an interface to the database that reveals as little as possible about what's happening under the hood. syncstorage-db has two features (mysql and spanner) that enable the MySQL and Spanner backends, respectively.

cargo features

I added two cargo features to the top-level syncserver crate: mysql and spanner. These features represent the two syncstorage database backends. This was an important addition because the Spanner backend pulls in protobuf and grpcio, which both contribute very substantially to the time it takes to build syncstorage. Giving developers the ability to omit the code related to Spanner when developing locally substantially improves compile times. By my measurements, compiling with the spanner feature takes about 4.5 minutes, whereas compiling with the mysql feature takes only about 1.5 minutes.

Adding these cargo features also reduces the size of the compiled binary, since we are omitting substantial amounts of code we never use.

Error Handling

The toughest part about this work was figuring out how to handle errors. Originally, we had a single DbError which encapsulated knowledge of both the Spanner and MySQL backends. Decoupling the Spanner and MySQL backend meant that each needed to have its own error type that included only knowledge of its own backend. To resolve this, I defined a number of different error types in different places and nested them using enums.

• syncserver-db-common:
  • MysqlError: An error representing low-level, MySQL-specific errors (e.g. connection errors, migration errors, errors that arise during queries, etc.). Basically, this wraps a few diesel errors. This type is shared by the syncstorage MySQL backend and the Tokenserver MySQL backend
• syncstorage-db-common:
  • SyncstorageDbError: An error that represents syncstorage-specific errors that arise during database queries. It represents things like CollectionNotFound, BsoNotFound, quota errors, batch errors, etc. This type doesn't wrap any lower-level errors, and the errors are not specific to MySQL or Spanner.
• syncstorage-mysql:
  • DbError: This is a top-level database error for the MySQL backend. It encapsulates both MysqlError and SyncstorageDbError, representing any error that may occur when processing a MySQL query for syncstorage.
• syncstorage-spanner:
  • DbError: This is a top-level database error for the Spanner backend. It encapsulates SyncstorageDbError and lower-level Spanner errors (there was no need for a SpannerError type akin to the MysqlError type, since the Tokenserver backend doesn't use Spanner). It represents any error that may occur when processing a Spanner query for syncstorage.
• tokenserver-mysql:
  • DbError: This is a top-level database error for tokenserver. It encapsulates MysqlError and any other internal errors that may occur when processing a MySQL query for tokenserver.

I also made the *ErrorKind types private. This was a more opinionated decision on my part, but IMO, it simplifies the public APIs of the error types, making them easier to understand and use by clients. I include a number of helper methods on the base error types to allow clients to access information hidden in the *ErrorKind types.

Use of debug_assertions

Unfortunately, it's not possible to pass through items defined conditionally under the test feature to other crates (i.e. if something is defined with #[cfg(test)] in a crate, it won't be rendered visible to parent crates). There are numerous testing utilities that we define under the test feature, and I thought it would be wrong to include them in release builds. To remedy this, I define testing utilities with the debug_assertions feature instead, which only compiles the testing utilities in debug builds. This feels a little inelegant, but I thought it would be preferable to including testing utilities in release builds or defining our own new testing feature, which would further complicate compiling syncserver.

CI

This PR introduces new cargo features (one for the Spanner backend and one for the MySQL backend), which are mutually exclusive. This makes it impossible to build a single Docker image against which we can run the MySQL and Spanner e2e tests. Instead, we need to build two separate Docker images: one compiled with the MySQL database backend and one compiled with the Spanner database backend. Our Docker image builds are quite lengthy, so I introduced cargo chef to facilitate the caching of build dependencies between CI runs. This dramatically reduces build times, since only our own crates need to be recompiled with code changes -- the dependencies are redownloaded and recompiled only if our Cargo.tomls change.

Additionally, I configured the MySQL and Spanner images/e2e tests to run in parallel, further improving build times. We might want to consider using a larger CircleCI resource class to allow cargo to build more crates in parallel and to improve single-threaded CPU performance.

Testing

Given that no functionality should be changing, ensuring compilation and that the unit and integration tests pass should be sufficient. We should definitely run through the end-to-end QA tests on stage before we deploy this to production.

Issue(s)

Closes #1277

[ethowitz]

This can't be merged until #1276 is merged

[ethowitz]

@pjenvey I like your idea about removing the Trait suffix and adding an Impl suffix to the types that implement the traits. I'll make the change!

[ethowitz]

CI was stuck so I did a git commit --amend --no-edit and force pushed, though I still see that GitHub appears to be waiting for the status of the e2e-tests job, which no longer exists in the CircleCI file...

[jrconlin]

I removed the requirement for the missing e2e-test from the master branch protection settings.

I am curious why they're missing.

[ethowitz]

@jrconlin The original e2e-tests job was removed and replaced with mysql-e2e-tests and spanner-e2e-tests -- should I update the branch protection to reflect that?

[jrconlin]

Ah, yeah. Sorry, forgot about that.

[ethowitz]

@jrconlin Ok I updated the branch protection settings, and that seems to have fixed the issue. I think this needs another approval before I can merge