wait for storage backup to finish to start new backup #12064
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
|
notebookStorageProvider calls autoSaveNotebookInHotExitFile on a 250ms debounce. But in that function it just does a fire and forget on storage.backup. If saving a big file (the one I was testing with was 70MB) these stack up faster than they can be resolved. So if you trace on the start and end of backup it looks like this if you just type away in a cell for a while, the hard drive is still spinning heavily for a while after finishing typing:
Instead of tweaking the debounce it made more sense to me to have the storage only allow for a single backup request to queue in line. Make the responsibility on the storage to not perform unneeded backups that have already been outdated by new incoming backup requests. Code now replaces the current waiting backup request with any new requests that come in. The same approximate amount of typing looks like this now: |
IanMatthewHuff
requested review from
DonJayamanne,
DavidKutu,
greazer,
joyceerhl,
rchiodo and
techwithtim
IanMatthewHuff
changed the title
| @@ -474,6 +474,12 @@ export class NativeEditorStorage implements INotebookStorage { | |||
| return this.savedAs.event; | |||
| } | |||
| private readonly savedAs = new EventEmitter<{ new: Uri; old: Uri }>(); | |||
|
|
|||
| // Keep track of if we are backing up our file already | |||
| private backingUp = false; | |||
There was a problem hiding this comment.
Another way to do this might be to cancel the previous save (create a merged token source) and debounce a new one every time a new request comes in and the previous isn't finished
There was a problem hiding this comment.
But then you might end up with half finished writes
There was a problem hiding this comment.
I wonder if part of the problem is we're writing the entire file every time. We could just do updates? That's a lot more complicated though. Probably have to keep a file stream open and seek around to the write spot and stuff.
There was a problem hiding this comment.
Yeah, I thought about the cancel as well. But my fear / worry was what you mentioned about the writes. The big time sink is the file write, and cancelling that halfway seems dicey with a write halfway done.
There was a problem hiding this comment.
Long term not writing the entire file seems like the right solution. I could open a new issue for that? I think it's a bigger work item. But I'd agree that writing a 70MB file shouldn't happen a few times a second as I type in a markdown cell. With some of these dense plots types I think that it's not too hard to generate pretty big ipynbs.
There was a problem hiding this comment.
Yeah the only real solution here would be to not write out the entire contents each time. I'll enter an issue for that.
For #12058
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).