Skip to content

Remove sqlite#25782

Closed
LK4D4 wants to merge 2 commits intomoby:masterfrom
LK4D4:remove_sqlite
Closed

Remove sqlite#25782
LK4D4 wants to merge 2 commits intomoby:masterfrom
LK4D4:remove_sqlite

Conversation

@LK4D4
Copy link
Copy Markdown
Contributor

@LK4D4 LK4D4 commented Aug 17, 2016

fixes #22146
relates to #16032

- What I did
Removed sqlite dependency.
- How I did it
Removed every usage of graphdb.
- How to verify it
Compile daemon
- Description for the changelog

Dependency from sqlite removed, so links upgrade from 1.9 does not work. External migration tool should be used if necessary.

- A picture of a cute animal (not mandatory but encouraged)
cat

I also wrote tool for migration: https://github.com/LK4D4/linksmigrator

@LK4D4 LK4D4 added this to the 1.13.0 milestone Aug 17, 2016
@aaronlehmann
Copy link
Copy Markdown

Shortly after we moved to Go 1.5, I looked into why compiles too so long. I found that a significant amount of the time was spent building sqlite. The Go compilation was parallelized, but the C compilation was not since sqlite is one big file.

Removing sqlite will probably have a beneficial impact on build times.

@aaronlehmann
Copy link
Copy Markdown

(That is, the time to build Docker, not the build subcommand.)

@justincormack
Copy link
Copy Markdown
Contributor

I remember we discussed this before, and there was some reluctance, but
definitely worth discussing again...

On 17 Aug 2016 5:59 a.m., "Aaron Lehmann" notifications@github.com wrote:

(That is, the time to build Docker, not the build subcommand.)


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#25782 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAdcPAuYvNQrkDyXCKQTD4MqwRhpn86mks5qgpU-gaJpZM4Jl_Yb
.

@thaJeztah
Copy link
Copy Markdown
Member

See this issue #22146

@thaJeztah
Copy link
Copy Markdown
Member

ping @cpuguy83 ptal

@cpuguy83
Copy link
Copy Markdown
Member

Oh man, major +1 here.
Thanks for writing the migration tool.

@thaJeztah
Copy link
Copy Markdown
Member

oh! I miss the migration tool, that is really awesome! Would be great if we had that under the "docker" organization - but guess we won't be able to ship it as an image (like the v1.10 migration tool). FWIW, once we're in docs review, I think we should have it added to https://github.com/docker/docker/blob/master/docs/migration.md

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 17, 2016

@thaJeztah I think we can incorporate that to migrator tool.

@vdemeester
Copy link
Copy Markdown
Member

I'm also definitely +1 on that 😻

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 17, 2016

@tonistiigi also pointed that we should notify people that they should run migration tool. Not sure how to make that behavior opt-out though.

@lowenna
Copy link
Copy Markdown
Member

lowenna commented Aug 17, 2016

Nice! @jstarks FYI

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 19, 2016

btw compile time goes from 50s to 40s on my machine without sqlite.

LK4D4 added 2 commits August 26, 2016 08:50
Signed-off-by: Alexander Morozov <lk4d4@docker.com>
Signed-off-by: Alexander Morozov <lk4d4@docker.com>
@cpuguy83 cpuguy83 added the status/needs-attention Calls for a collective discussion during a review session label Sep 8, 2016
@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Sep 8, 2016

I guess my only real concern is that there is still a large contingent of people running docker 1.9 due to build caching issues in >= 1.10.
How many of those people are using --link that would be affected by this?

@thaJeztah
Copy link
Copy Markdown
Member

@cpuguy83 what do you think? Should we move this to 1.14? Or don't do it at all?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Sep 22, 2016

@cpuguy83 or shall we document that people from 1.9 should always use 1.10, perhaps we should check if we document that we support upgrading and skipping versions?

If so, then I think we can move forward

@cpuguy83
Copy link
Copy Markdown
Member

@thaJeztah Do people read docs before upgrading?
I'm just worried we are going to break people, especially when there are so many on 1.9 still.

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Sep 27, 2016

@thaJeztah @cpuguy83 yeah, let's wait for a release with caching improvements, so people can upgrade. I'll close this for now.

@LK4D4 LK4D4 closed this Sep 27, 2016
@thaJeztah
Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/needs-attention Calls for a collective discussion during a review session status/1-design-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFC: deprecate old graphdb migration / move to an external tool?

7 participants