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

metadb: split backends into packages, add metadb #151

Merged
merged 3 commits into from
Feb 11, 2021
Merged

Conversation

tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Feb 11, 2021

The old structure was a single huge package with all the DB backends in
it. Since tm-db users often want just one or two DB backends, and
building plus linking all of them is very heavy, build tags were used to
opt into some of the backends.

This same mechanism is now in metadb, for those who want it; it allows
opting into DB backends via build tags, without having to edit the code
directly.

Note that the old package included the memdb and goleveldb backends by
default. The new metadb does not include any by default, to make
backends explicit and more consistent. This required using more build
tags in CI.

Most users will want to statically use one DB implementation, without
metadb. Each separate sub-package does just that. They are direct moves
of the old code; the only major change is that constructors like
db.NewBadgerDB are now called badger.NewDB, to avoid the repetition of
badger.NewBadgerDB.

The root package error types are also exported now, to allow being used
by all separate packages. Note that this means they're now part of the
public API too; this should be fine.

Some test code also had to be shuffled around. All the tests which dealt
with backends were moved to metadb. The shared test helper code had to
be moved to an internal package, to remain reachable.

Finally, note that we consistently use "tmdb" as the imported package
name for the root package, since "db" is often used as a variable name.
This prevents confusion and common build errors.

Fixes #113.

Thank you @mvdan for the contribution.

mvdan and others added 2 commits February 9, 2021 17:23
The old structure was a single huge package with all the DB backends in
it. Since tm-db users often want just one or two DB backends, and
building plus linking all of them is very heavy, build tags were used to
opt into some of the backends.

This same mechanism is now in metadb, for those who want it; it allows
opting into DB backends via build tags, without having to edit the code
directly.

Note that the old package included the memdb and goleveldb backends by
default. The new metadb does not include any by default, to make
backends explicit and more consistent. This required using more build
tags in CI.

Most users will want to statically use one DB implementation, without
metadb. Each separate sub-package does just that. They are direct moves
of the old code; the only major change is that constructors like
db.NewBadgerDB are now called badger.NewDB, to avoid the repetition of
badger.NewBadgerDB.

The root package error types are also exported now, to allow being used
by all separate packages. Note that this means they're now part of the
public API too; this should be fine.

Some test code also had to be shuffled around. All the tests which dealt
with backends were moved to metadb. The shared test helper code had to
be moved to an internal package, to remain reachable.

Finally, note that we consistently use "tmdb" as the imported package
name for the root package, since "db" is often used as a variable name.
This prevents confusion and common build errors.

Fixes #113.
@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #151 (26d17c4) into master (6f9a08c) will decrease coverage by 57.40%.
The diff coverage is 13.15%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #151       +/-   ##
===========================================
- Coverage   68.78%   11.38%   -57.41%     
===========================================
  Files          27       31        +4     
  Lines        1448     1309      -139     
===========================================
- Hits          996      149      -847     
- Misses        377     1127      +750     
+ Partials       75       33       -42     
Impacted Files Coverage Δ
boltdb/batch.go 0.00% <0.00%> (ø)
boltdb/iterator.go 0.00% <ø> (ø)
cleveldb/batch.go 0.00% <0.00%> (ø)
cleveldb/db.go 0.00% <0.00%> (ø)
cleveldb/iterator.go 0.00% <0.00%> (ø)
goleveldb/batch.go 0.00% <0.00%> (ø)
goleveldb/iterator.go 0.00% <0.00%> (ø)
memdb/batch.go 0.00% <0.00%> (ø)
memdb/db.go 0.00% <0.00%> (ø)
memdb/iterator.go 0.00% <ø> (ø)
... and 42 more
Impacted Files Coverage Δ
boltdb/batch.go 0.00% <0.00%> (ø)
boltdb/iterator.go 0.00% <ø> (ø)
cleveldb/batch.go 0.00% <0.00%> (ø)
cleveldb/db.go 0.00% <0.00%> (ø)
cleveldb/iterator.go 0.00% <0.00%> (ø)
goleveldb/batch.go 0.00% <0.00%> (ø)
goleveldb/iterator.go 0.00% <0.00%> (ø)
memdb/batch.go 0.00% <0.00%> (ø)
memdb/db.go 0.00% <0.00%> (ø)
memdb/iterator.go 0.00% <ø> (ø)
... and 42 more

@tac0turtle tac0turtle marked this pull request as ready for review February 11, 2021 16:04
@tac0turtle tac0turtle changed the title metadb: ci fixes metadb: split backends into packages, add metadb Feb 11, 2021
@tac0turtle tac0turtle self-assigned this Feb 11, 2021
@tac0turtle tac0turtle added the S:automerge Status: Auto merge a pull request label Feb 11, 2021
@tac0turtle tac0turtle requested a review from cmwaters February 11, 2021 16:11
@tac0turtle tac0turtle merged commit 71bc5d8 into master Feb 11, 2021
@tac0turtle tac0turtle deleted the metadb-ci branch February 11, 2021 16:19
@tac0turtle
Copy link
Contributor Author

So the tests pass here but not on master, will dig into it

@klim0v
Copy link
Contributor

klim0v commented Sep 2, 2021

I think this change should have tag v0.7.0

@tac0turtle
Copy link
Contributor Author

you are correct! ill see if the tag is still around

creachadair added a commit that referenced this pull request Nov 8, 2021
creachadair pushed a commit that referenced this pull request Nov 9, 2021
creachadair added a commit that referenced this pull request Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:automerge Status: Auto merge a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

current package layout hurts compile time and binary sizes
4 participants