Adjust error for already locked db and prevent level db lock on malformed connstr #18923
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rmed connstr This PR adjusts the error returned when there is failure to lock the level db, and permits a connections to the same leveldb where there is a different connection string. Reference go-gitea#18921 Reference go-gitea#18917 Signed-off-by: Andrew Thornton <[email protected]>
|
is backportable |
GiteaBot
added
the
lgtm/need 2
Gusted
approved these changes
Feb 26, 2022
GiteaBot
added
lgtm/need 1
6543
approved these changes
Feb 26, 2022
GiteaBot
added
lgtm/done
wxiaoguang
reviewed
Feb 27, 2022
Comment on lines
+73
to
+74
| db.name = append(db.name, connection) | ||
| m.LevelDBConnections[connection] = db |
There was a problem hiding this comment.
Suggested change
| db.name = append(db.name, connection) | |
| m.LevelDBConnections[connection] = db |
If we run into this if, then neither connection nor uri exists in the LevelDBConnections.
But here the code only append the connection to the name list, but no uri, is it by purpose?
In my opinion:
- Either we keep consistent that we append both non-existing
connectionanduriinto the name list - Or append nothing (remove these 2 lines), because the logic is sill correct, no performance degrade, and more simple.
Since the old code works, this comment is not a blocker.
There was a problem hiding this comment.
It prevents the repeated logging of the same message
There was a problem hiding this comment.
But it's not complete.
Current code It only prevents repeated logging for the new connection, but not for the new uri with the same directory. (that the case 1)
And I do not think there will be repeated loggings, the queues call GetLevelDB for open the db, there seems no repeated calls (so that why case 2 is also fine because there seems to be no repeated log messages)
There was a problem hiding this comment.
Yes it absolutely does prevent repeated logging of the same message. The log message doesn't contain the inferred URI. The point is to tell the server user which strings in their configuration are incorrect.
There are multiple queues. Depending on the order of initialisation and state of the incorrect configuration without this you face repeatedly seeing the same warning message.
|
|
The error |
Signed-off-by: Andrew Thornton <[email protected]>
|
I've adjusted the CloseLevelDB to also look for the datadir too. |
|
make lgtm work |
zeripath
added a commit
to zeripath/gitea
that referenced
this pull request
Feb 27, 2022
…rmed connstr (go-gitea#18923) Backport go-gitea#18923 This PR adjusts the error returned when there is failure to lock the level db, and permits a connections to the same leveldb where there is a different connection string. Reference go-gitea#18921 Reference go-gitea#18917 Signed-off-by: Andrew Thornton <[email protected]>
zeripath
added
backport/done
zjjhot
added a commit
to zjjhot/gitea
that referenced
this pull request
Feb 28, 2022
* giteaofficial/main: Adjust error for already locked db and prevent level db lock on malformed connstr (go-gitea#18923) Avoid database lookups for `DescriptionHTML` (go-gitea#18924) Update go-org to v1.6.1 (go-gitea#18932) Fix trace log to show value instead of pointers (go-gitea#18926) Add announcement steps to release-howto (go-gitea#18896) Fix `<strong>` html in translation (go-gitea#18929)
zeripath
added a commit
that referenced
this pull request
Feb 28, 2022
…rmed connstr (#18923) (#18938) Backport #18923 This PR adjusts the error returned when there is failure to lock the level db, and permits a connections to the same leveldb where there is a different connection string. Reference #18921 Reference #18917 Signed-off-by: Andrew Thornton <[email protected]>
Chianina
pushed a commit
to Chianina/gitea
that referenced
this pull request
Mar 28, 2022
…rmed connstr (go-gitea#18923) This PR adjusts the error returned when there is failure to lock the level db, and permits a connections to the same leveldb where there is a different connection string. Reference go-gitea#18921 Reference go-gitea#18917 Signed-off-by: Andrew Thornton <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adjusts the error returned when there is failure to lock the level db, and
permits a connections to the same leveldb where there is a different connection string.
Reference #18921
Reference #18917
Signed-off-by: Andrew Thornton [email protected]