Add dbless persistence #8670
Add dbless persistence #8670
Conversation
aboudreault
force-pushed
the
feat/dbless-persistence
branch
from
18dc7d1 to
155a546
Compare
aboudreault
force-pushed
the
feat/dbless-persistence
branch
from
155a546 to
6b02995
Compare
aboudreault
force-pushed
the
feat/dbless-persistence
branch
from
6b02995 to
c595b35
Compare
|
@bungle is it possible for you to take a closer look at this (to make sure we are not overlooking anything)? Especially make sure the reload doesn't mess up anything. |
aboudreault
added
the
pr/ready
| assert.res_status(404, res) -- 404, only the declarative config is loaded | ||
| end) | ||
|
|
||
| it("doesn't load the persisted lmdb config if a declarative config is set on reload", function() |
There was a problem hiding this comment.
For good measure, I think it would be worthwhile to have a test case that covers worker process replacement (kill -9 an existing worker and then validate expected behavior) as well.
There was a problem hiding this comment.
@flrgh would you consider this test a blocker? I thought that it will basically do the same thing than an initial worker start, cause the code is right in init_worker.
There was a problem hiding this comment.
Nah I wouldn't go as far as to say it's a blocker. I don't know this code as well as you do, so I think this is up to your discretion whether more tests are needed before merging. To clarify, there's nothing in particular about this changeset that makes me believe it will introduce any new bugs. I just think our test coverage of single worker replacement is lacking overall (it's a very common blind spot I've observed in OpenResty code), so I'll almost always advocate for adding more tests when init_worker is concerned.
There was a problem hiding this comment.
makes sense @flrgh! I will check to add one before merging.
There was a problem hiding this comment.
@flrgh since I am working on other tasks and we want to unblock as many PR merges as possible before the feature freeze, I will merge this now, but still consider your test case addition is worth to add.
aboudreault
force-pushed
the
feat/dbless-persistence
branch
from
058f19a to
dc1f746
Compare
This reverts commit c3fbd61.
This reverts commit c3fbd61.
This reverts commit 0c1e2eb.
…""" This reverts commit cf1a91b.
…""" This reverts commit cf1a91b.
This PR makes the dbless persistent between restarts (lmdb). If a config exists in the db, load it and build the router.
With this addition, we can remove the config cache (config.cache.tar.gz) (I will open another PR for this).