feat: make staleness check more robust #74
Conversation
now we keep track of the mtime updated by the current process, and if we lose some cycles in the update process but recover and the mtime is still ours we do not mark the lock as compromised
lib/lockfile.js
Outdated
| // Check if mtime is still ours | ||
| options.fs.stat(getLockFile(file, options), (err, stat) => { | ||
| // Check mtime ownership | ||
| const isMtimeOurs = options.checkMtimeOwnership && lock.stat.mtimeMs === (stat ? stat.mtimeMs : null); |
There was a problem hiding this comment.
I would not make this an option and just be enabled by default.
There was a problem hiding this comment.
this is just for testing and it is enabled by default
|
@hugomrdias CI is failing for some reason, can you also check that? |
Codecov Report
@@ Coverage Diff @@
## master #74 +/- ##
==========================================
- Coverage 100% 98.21% -1.79%
==========================================
Files 4 4
Lines 161 168 +7
Branches 39 41 +2
==========================================
+ Hits 161 165 +4
- Misses 0 3 +3
Continue to review full report at Codecov.
|
|
Feel free to merge! Don’t forget to add the (possible) breaking change in the commit message. |
|
@hugomrdias do you mind doing |
lib/lockfile.js
Outdated
| // which we consider a valid case | ||
| // Check if mtime is still ours if it is we can still recover from a system sleep or a busy event loop | ||
| options.fs.stat(getLockFile(file, options), (err, stat) => { | ||
| const isMtimeOurs = lock.mtime.getTime() === (stat ? stat.mtime.getTime() : null); |
There was a problem hiding this comment.
Nit: personally I'd move this after the error handling block so you don't need to use a ternary 😄
lib/lockfile.js
Outdated
| return setLockAsCompromised(file, lock, Object.assign(err, { code: 'ECOMPROMISED' })); | ||
| } | ||
|
|
||
| lock.updateError = err; |
There was a problem hiding this comment.
These are assigned to but never used...
|
Ok this is working great for me! 👌 |
now we keep track of the mtime updated by the current process, and if we lose some cycles in the update process but recover and the mtime is still ours we do not mark the lock as compromised
closes #71
ref:
ipfs/js-ipfs-repo#188 (comment)