Make applyRemoteEvent idempotent #2263
Conversation
| }); | ||
|
|
||
| // Reset newQueryDataByTargetMap in case this transaction gets re-run. | ||
| newQueryDataByTargetMap = this.queryDataByTarget; |
There was a problem hiding this comment.
FYI: The only change in this block is line 486 to 491 and the fact that everything below uses newQueryDataByTargetMap.
| export interface ActiveTargets { | ||
| [id: number]: unknown; | ||
| } | ||
| export type ActiveTargets = SortedMap<TargetId, unknown>; |
There was a problem hiding this comment.
Changed this type so I could continue to parse the target map from LocalStore.
schmidt-sebastian
changed the title
| @@ -31,30 +30,17 @@ import { SimpleDbStore } from './simple_db'; | |||
| * A persisted implementation of IndexManager. | |||
| */ | |||
| export class IndexedDbIndexManager implements IndexManager { | |||
| /** | |||
There was a problem hiding this comment.
I can't find where this cache is kept now. Unconditionally writing it would be a pretty major regression.
Or is the equivalent supposed to be the collectionParents addition in indexeddb_mutation_queue.ts? This cache ensures that we only ever write a collection parent index entry once for the entire duration of the app. The indexeddb_mutation_queue.ts changes ensure uniqueness within a mutation batch, but that's a far lesser benefit.
On the whole I think unwinding all of this in the name of idempotency is a bad deal. If we added transaction.onCommitted this wouldn't be impacted at all. This is the second case we were looking for--I think it makes sense to pursue that instead of these changes.
schmidt-sebastian
force-pushed
the
mrschmidt/applyremoteevent
branch
from
414beec to
2843d42
Compare
|
I am not sure why this doesn't compile according to Travis (I will check after 1 pm), but I modified this PR to pull in the latest changes. Unfortunately, I still had to keep some of the new logic that determines whether we have already seen a collection parent before we write it, since the accounting now only happens after a transaction commit. We might be able to clean this up more if we add a |
* Add transaction retries (#2250) * Marking SimpleDb calls as idempotent (#2251) * Mark mostly readonly calls as idempotent (#2252) * Fix test failure (#2256) * Make handleUserChange idempotent (#2257) * Temporarily disable CountingQueryEngine tests (#2258) * Improve test hack (#2259) * Improve test hack * Comment in test hack * Make getNewDocumentChanges() idempotent (#2255) * Add onCommitted listeners for transactions (#2265) * Fix build * Fix Lint * Make applyRemoteEvent idempotent (#2263) * Make notifyLocalViewChanges idempotent (#2268) * Make releaseQuery idempotent (#2266) * Mark acknowledgeBatch and rejectBatch idempotent (#2269) * idempotent `allocateQuery` and `notifyLocalViewChanges` (#2264) * Mark collectGarbage idempotent (#2267) * Idempotency: Address TODOs, add Changelog (#2270)
This one was the most "fun" so far.
Major changes: