-
-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Implement worker threads for upload processing #1591
Conversation
This reverts commit 322120f.
Using the file system location is much more reliable than passing around buffer.
@@ -61,7 +61,12 @@ class DownloadController { | |||
return; | |||
} | |||
|
|||
const page = DownloadPage({ id, files }); | |||
console.log('files', files); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove debug log
Do not copy apkg when not used. Due to the thread changes we do not to keep the buffer in memory.
const page = DownloadPage({ | ||
id, | ||
files: files.filter((file) => file.endsWith('.apkg')), | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do not want to index images and other files
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Unfortunately the node worker threads cannot share buffers reliably with the main thread. I learned this the hard way 😅
So I have move all of the read writes of the APKG payload to be be based on file system access which is thread safe since we only read after writes.
While reviewing the code path, I discovered that we actually do a lot of wasteful reads and writes. Essentially for every APKG generation we make 2x calls due to the renaming to provide the uploader with friendly name. I have not measured how long this takes but instead of double work we do this one time and rely on the create_deck having the right name.
Revert of revert #1589
Reference: 2anki/create_deck#93