Build names and links at runtime - no more sqlite#16032
Conversation
cpuguy83
force-pushed
the
remove_sqlite_dep
branch
from
a94aa9c to
77fb28a
Compare
cpuguy83
force-pushed
the
remove_sqlite_dep
branch
2 times, most recently
from
102a747 to
7283b4e
Compare
|
phew, these rebases are rough. |
|
And realizing that hostconfig.Links was being set to nil before, so this information is only in sqlite... |
|
−158,972 |
hack/make/validate-lint
Outdated
|
Looks cool |
|
Wow this is epic. Thanks so much @cpuguy83 ! Is there any undesirable side effect you can think of? |
|
Maybe an obvious question, but does this mean that links won't survive a daemon restart, or were they still stored on-disk somewhere else besides SQLite? |
|
Adding some context this from IRC convo: |
|
Confirmed that this works with Windows containers (both existing images after updating just docker.exe and on a fresh install). Not sure why Windows Jenkins run is failing, but it works fine in manual tests. LGTM |
cpuguy83
force-pushed
the
remove_sqlite_dep
branch
from
7283b4e to
faf92dd
Compare
|
ping @mavenugo @mrjana could you have a look at #16032 (comment) now that 1.9 has been release (and you hopefully have a bit more time ❤️) |
|
we cannot remove links from one version to the other. If we're really going to remove links we should deprecated them in 1.10 to completely remove sqlite3 and the code related in 1.12. If we're going to keep links as they are, we can make the transition to in-memory links in 1.10, loading from sqlite to memory in that version, and remove the dependency in 1.11. If we're going to keep links with a different implementation under the hood, we should make sure we remove sqlite from the dependencies. |
|
@calavera unfortunately we are stuck with sqlite in any case since |
|
@cpuguy83 should we close this for now or is this still an option? I see a discussion around removal of links, which is not yet clear when that'll happen |
|
@thaJeztah The overall goal is still valid, don't use sqlite for storing things on disk which we can build at runtime easily enough. |
|
@cpuguy83 need rebase |
cpuguy83
force-pushed
the
remove_sqlite_dep
branch
from
faf92dd to
3525c75
Compare
|
@LK4D4 Not yet. |
cpuguy83
force-pushed
the
remove_sqlite_dep
branch
3 times, most recently
from
aeeb1c6 to
5c22bf2
Compare
cpuguy83
force-pushed
the
remove_sqlite_dep
branch
from
c92aeba to
2600777
Compare
|
LGTM |
|
LGTM |
Build names and links at runtime - no more sqlite
|
ping @ibuildthecloud aka, demolisher of code, hopes, and dreams. |
|
Yay, merged! Thanks for being so patient @cpuguy83 🤘 |
|
Is this the candidate fix to #17720? If this is on experimental I'm down to see if it'll still lockup on my production boxes. |
|
@vincentwoo This definitely speeds up some pieces of |
|
I can give it a run. Is it on https://experimental.docker.com? I have a hard time figuring out what revision that thing is at. |
|
This is in stable. |
|
Right, but there's not binaries out against it yet, right? Slated for 1.10? |
|
Niiiice. |
Before moby#16032, once links were setup in the sqlite db, hostConfig.Links was cleared out. This means that we need to migrate data back out of the sqlite db and put it back into hostConfig.Links so that links specified on older daemons can be used. Signed-off-by: Brian Goff <[email protected]>
See GitHub issue: moby#17691 for details. Solution is in: moby#16032 This commit cherry-picks 0f9f995 and applies it to the cr-defunct branch. Build names and links at runtime Don't rely on sqlite db for name registration and linking. Instead register names and links when the daemon starts to an in-memory store.
(*Daemon).registerLinks() calling the WriteHostConfig() method of its container argument is a vestigial behaviour. In the distant past, registerLinks() would persist the container links in an SQLite database and drop the link config from the container's persisted HostConfig. This changed in Docker v1.10 (moby#16032) which migrated away from SQLite and began using the link config in the container's HostConfig as the persistent source of truth. registerLinks() no longer mutates the HostConfig at all so persisting the HostConfig to disk falls outside of its scope of responsibilities. Signed-off-by: Cory Snider <[email protected]>
(*Daemon).registerLinks() calling the WriteHostConfig() method of its container argument is a vestigial behaviour. In the distant past, registerLinks() would persist the container links in an SQLite database and drop the link config from the container's persisted HostConfig. This changed in Docker v1.10 (moby#16032) which migrated away from SQLite and began using the link config in the container's HostConfig as the persistent source of truth. registerLinks() no longer mutates the HostConfig at all so persisting the HostConfig to disk falls outside of its scope of responsibilities. Signed-off-by: Cory Snider <[email protected]> (cherry picked from commit 388fe4a) Signed-off-by: Cory Snider <[email protected]>
11 participants
This stores all names and link info in memory at runtime rather than relying on sqlite.
Please test thoroughly.
I added some tests, but this is a big change!
Fixes #17691