lib/protocol: Cache expensive key operations (fixes #8599) #8820
Conversation
This file contains hidden or 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
This adds a cache to the expensive key generation operations. It's fixes size LRU/MRU stuff to keep memory usage bounded under absurd conditions. Also closes syncthing#8600.
* main: build: Update dependencies
* main: gui: Add Persian (fa) translation template (syncthing#8822) lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563) gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539) build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804) build: Update dependencies (syncthing#8821)
imsodin
reviewed
Mar 11, 2023
lib/protocol/encryption.go
Outdated
| cache *lru.TwoQueueCache[folderKeyGeneratorCacheKey, *[keySize]byte] | ||
| } | ||
|
|
||
| var DefaultFolderKeyGenerator FolderKeyGenerator |
Member
There was a problem hiding this comment.
I built a bit of a global-aversion during some past changes - don't even remember what they were, but the aversion stayed. I'd like to store this in connection.service and pass it to the protocol connection constructors. I am happy to propose such a change, as I am asking for (maybe even not that little) busywork/refactoring with not very strong arguments :)
Let me know if you'd appreciate that.
Member
Author
There was a problem hiding this comment.
There you go... It's a fairly infectious change, and as I'm now touching one hundred files you get some spurious gofumpt changes as well.
Member
There was a problem hiding this comment.
Yeah, that infectiousness was what I was suspecting... Thanks a lot!
And wow, it's coming all the way from the top o.O
I think I'd have taken the lazier route and created it inside connection.service :)
Member
Author
There was a problem hiding this comment.
Can't do that, because the model also needs it for constructing tokens. That could have been two separate instances because they only hold a cache and some duplication would be OK, but if we ever make the key generation instance smarter for whatever reason that would be a footgun...
imsodin
approved these changes
Mar 12, 2023
| func writeEncryptionToken(token []byte, cfg config.FolderConfiguration) error { | ||
| tokenName := encryptionTokenPath(cfg) | ||
| fd, err := cfg.Filesystem(nil).OpenFile(tokenName, fs.OptReadWrite|fs.OptCreate, 0666) | ||
| fd, err := cfg.Filesystem(nil).OpenFile(tokenName, fs.OptReadWrite|fs.OptCreate, 0o666) |
Member
There was a problem hiding this comment.
TIL: 0o notation exists for octal.
calmh
added a commit
to calmh/syncthing
that referenced
this pull request
Mar 12, 2023
* main: lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820) gui: Add Persian (fa) translation template (syncthing#8822) lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563) gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539) build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804) build: Update dependencies (syncthing#8821)
calmh
added a commit
to tomasz1986/syncthing
that referenced
this pull request
Mar 18, 2023
* main: (424 commits) gui, man, authors: Update docs, translations, and contributors lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820) gui: Add Persian (fa) translation template (syncthing#8822) lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563) gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539) build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804) build: Update dependencies (syncthing#8821) lib/api: Expose `blocksHash` in file info (syncthing#8810) gui, man, authors: Update docs, translations, and contributors lib/discover: Don't leak relay-tokens to discovery (syncthing#8762) gui, man, authors: Update docs, translations, and contributors gui: Add Croatian (hr) translation template (syncthing#8801) build(deps): bump github.com/quic-go/quic-go from 0.32.0 to 0.33.0 (syncthing#8800) cmd/stupgrades: Cache should apply to HEAD as well as GET build: Add more GitHub Actions gui: Remove non-existent customicons.css file reference (fixes syncthing#8797) (syncthing#8798) Only fail after chmod error if permissions differ (e.g. on config file) (syncthing#8771) gui, man, authors: Update docs, translations, and contributors build: Use Go 1.19.6 for Windows build: Update dependencies ...
calmh
added a commit
to calmh/syncthing
that referenced
this pull request
Apr 14, 2023
* main: (28 commits) build: Update dependencies (syncthing#8869) lib/model: Improve path generation for auto accepted folders (fixes syncthing#8859) (syncthing#8860) docs: fix typo (syncthing#8857) gui, man, authors: Update docs, translations, and contributors lib/syncthing: Handle successful global migration (fixes syncthing#8851) (syncthing#8852) gui, man, authors: Update docs, translations, and contributors lib/model: Set enc. trailer size on pull (ref syncthing#8563, syncthing#8556) (syncthing#8839) lib/model: Fix file size inconsistency due to enc. trailer (syncthing#8840) gui, man, authors: Update docs, translations, and contributors cmd/stdiscorv: Fix database test (fixes syncthing#8828) lib/ur: Fix custom releases URL comparison gui: Remove untranslated strings from JSON files (syncthing#8836) all: Fix typos found by codespell (syncthing#8833) gui: Hide download progress legend when download progress is disabled gui, man, authors: Update docs, translations, and contributors lib/protocol: Handle encrypted requests without encrypted hash (fixes syncthing#8277) (syncthing#8827) build(deps): bump github.com/hashicorp/golang-lru/v2 from 2.0.1 to 2.0.2 (syncthing#8826) lib/config: Allow sub-second watcher delay (fixes syncthing#7859) (syncthing#7864) gui, man, authors: Update docs, translations, and contributors lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820) ...
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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 adds a cache to the expensive key generation operations. It's fixes size LRU/MRU stuff to keep memory usage bounded under absurd conditions.
Also closes #8600.