Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crdbtest: move crdb-specific code to a lib #4099

Open
RaduBerinde opened this issue Oct 22, 2024 · 0 comments
Open

crdbtest: move crdb-specific code to a lib #4099

RaduBerinde opened this issue Oct 22, 2024 · 0 comments

Comments

@RaduBerinde
Copy link
Member

RaduBerinde commented Oct 22, 2024

The CRDB-specific implentations of Comparer and KeySchema are defined in the CRDB tree, as well as the internal/crdbtest subpackage. It would be ideal to have a single implementation so we don't have to rely on error-prone manual porting of changes.

The current proposal is to move it into a top-level directory inside pebble, along with a linter that disallows importing it from non-test code. CRDB would import it and use it directly.

We have discussed putting it into a third repository, but it adds some unnecessary complications;

  • the dependency story is not clean (unless we remove all crdb-related test stuff from the pebble repo)
  • making changes would be harder to synchronize.
  • bumping the pebble and the third library dependency in CRDB will be more complicated

There is a small hiccup with addressing this issue, related to test-only checks: the CRDB version uses the crdb-specific buildutil.CrdbTestBuild and Pebble is currently never built with the invariant tag as part of the CRDB bazel build. Because of this difficulty, we should aim to do this only on master and leave release-24.3 as is.

Jira issue: PEBBLE-284

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

2 participants