Page MenuHomePhabricator
Differential D14457

[lib] Create unshimming queue
ClosedPublic
Actions

Authored by tomek on Mon, Mar 17, 7:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 17, 8:25 AM
Unknown Object (File)
Mon, Mar 17, 8:23 AM
Unknown Object (File)
Mon, Mar 17, 8:18 AM
Subscribers
ashoat

Details

Reviewers
kamil
bartek
Commits
rCOMMfe3542bad7fc: [lib] Create unshimming queue
Summary

The unshimming process happens during a migration when it isn't convenient to call e.g. processDMOperations. To make this easier, we're saving the operations in Redux. We're processing them using a queue so that we don't have issues with e.g. Redux changes being skipped. We're removing the operations from SQLite only after they are processed.

https://linear.app/comm/issue/ENG-10329/create-a-new-action-and-modify-the-reducedmoperationsqueue-reducer

Depends on D14456

Test Plan

Tested the whole stack on both native and web:

  1. Run the app on Redux version 86 with a modification to processDMOperation making all the INBOUND messages unsupported
  2. On one platform created a thread and sent a text message. These were visible on the sender, and invisible on the recipient
  3. Closed the recipient and added a migration that unshimms the thread creation operations, e.g.
unshimDMOperations(
  state,
  dmOperationTypes.CREATE_THREAD,
  handleReduxMigrationFailure,
): MigrationFunction<WebNavInfo, AppState>),
  1. Run the recipient and noticed the thread appearing
  2. Added another migration, this time unshimming text messages
  3. Run the recipient and noticed messages appearing

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Mon, Mar 17, 7:10 AM
Herald added a subscriber: ashoat. · View Herald TranscriptMon, Mar 17, 7:10 AM
tomek added a child revision: D14458: [lib] Create a migration function.Mon, Mar 17, 7:13 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Mon, Mar 17, 7:47 AM
Harbormaster failed remote builds in B33733: Diff 47422!
tomek updated this revision to Diff 47449.Tue, Mar 18, 3:42 AM

Rebase

Harbormaster completed remote builds in B33759: Diff 47449.Tue, Mar 18, 4:06 AM
tomek requested review of this revision.Tue, Mar 18, 4:06 AM
kamil accepted this revision.Tue, Mar 18, 9:05 AM

Good idea not unshimming in a migration

lib/shared/dm-ops/dm-ops-queue-handler.react.js
284

This is never set false to make sure we attempt unshimming only once after app start? If so, might be good to add code coment

285–302

I would use flatMap() but this is fine too

287–301

My memory about queue model we have is rusty, but I assume it is safe to perform operation and separately dispatch action? (I am talking about case when e.g. operation could not be processed)

This revision is now accepted and ready to land.Tue, Mar 18, 9:05 AM
tomek updated this revision to Diff 47457.Wed, Mar 19, 6:33 AM

Enqueue operations directly

lib/shared/dm-ops/dm-ops-queue-handler.react.js
284

This is never set false to make sure we attempt unshimming only once after app start?

Yes, exactly

285–302

I think we should replace this code a bit to follow patterns from this file.

287–301

Yes, this is the same pattern we're using in other places. Both operation and removing it could fail, but there isn't too much we can do in such cases.

Harbormaster completed remote builds in B33765: Diff 47457.Wed, Mar 19, 6:56 AM
Closed by commit rCOMMfe3542bad7fc: [lib] Create unshimming queue (authored by tomek). · Explain WhyWed, Mar 19, 9:57 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMfe3542bad7fc: [lib] Create unshimming queue.

Revision Contents
Changeset List

PathSize
lib/
reducers/
18 lines
shared/
dm-ops/
33 lines
types/
7 lines
5 lines
utils/
1 line
native/
redux/
1 line
12 lines
web/
redux/
1 line
2 lines
10 lines

Diff 47449

View Options

lib/reducers/dm-operations-queue-reducer.js

// @flow // @flow
import _mapValues from 'lodash/fp/mapValues.js'; import _mapValues from 'lodash/fp/mapValues.js';
import type { DMOperationStoreOperation } from '../ops/dm-operations-store-ops.js'; import type { DMOperationStoreOperation } from '../ops/dm-operations-store-ops.js';
import { import {
clearQueuedEntryDMOpsActionType, clearQueuedEntryDMOpsActionType,
clearQueuedMembershipDMOpsActionType, clearQueuedMembershipDMOpsActionType,
clearQueuedMessageDMOpsActionType, clearQueuedMessageDMOpsActionType,
clearQueuedThreadDMOpsActionType, clearQueuedThreadDMOpsActionType,
type OperationsQueue, type OperationsQueue,
pruneDMOpsQueueActionType, pruneDMOpsQueueActionType,
type QueuedDMOperations, type QueuedDMOperations,
queueDMOpsActionType, queueDMOpsActionType,
reportUnshimmingOperationCompletedActionType,
saveUnsupportedOperationActionType, saveUnsupportedOperationActionType,
} from '../types/dm-ops.js'; } from '../types/dm-ops.js';
import type { BaseAction } from '../types/redux-types.js'; import type { BaseAction } from '../types/redux-types.js';
function reduceDMOperationsQueue( function reduceDMOperationsQueue(
store: QueuedDMOperations, store: QueuedDMOperations,
action: BaseAction, action: BaseAction,
): { ): {
▲ Show 20 Lines • Show All 160 Lines • ▼ Show 20 Lines return {
payload: { payload: {
id: action.payload.id, id: action.payload.id,
type: action.payload.operation.type, type: action.payload.operation.type,
operation: action.payload.operation, operation: action.payload.operation,
}, },
}, },
], ],
}; };
} else if (action.type === reportUnshimmingOperationCompletedActionType) {
return {
store: {
...store,
shimmedOperations: store.shimmedOperations.filter(
op => op.id !== action.payload.id,
),
},
operations: [
{
type: 'remove_dm_operations',
payload: {
ids: [action.payload.id],
},
},
],
};
} }
return { store, operations: [] }; return { store, operations: [] };
} }
export { reduceDMOperationsQueue }; export { reduceDMOperationsQueue };
View Options

lib/shared/dm-ops/dm-ops-queue-handler.react.js

Show All 10 Linesimport {
threadInfoSelector, threadInfoSelector,
} from '../../selectors/thread-selectors.js'; } from '../../selectors/thread-selectors.js';
import { import {
clearQueuedEntryDMOpsActionType, clearQueuedEntryDMOpsActionType,
clearQueuedMembershipDMOpsActionType, clearQueuedMembershipDMOpsActionType,
clearQueuedMessageDMOpsActionType, clearQueuedMessageDMOpsActionType,
clearQueuedThreadDMOpsActionType, clearQueuedThreadDMOpsActionType,
pruneDMOpsQueueActionType, pruneDMOpsQueueActionType,
reportUnshimmingOperationCompletedActionType,
} from '../../types/dm-ops.js'; } from '../../types/dm-ops.js';
import type { DMOperation } from '../../types/dm-ops.js'; import type { DMOperation } from '../../types/dm-ops.js';
import type { BaseAction } from '../../types/redux-types.js'; import type { BaseAction } from '../../types/redux-types.js';
import { useDispatch, useSelector } from '../../utils/redux-utils.js'; import { useDispatch, useSelector } from '../../utils/redux-utils.js';
const PRUNING_FREQUENCY = 60 * 60 * 1000; const PRUNING_FREQUENCY = 60 * 60 * 1000;
const FIRST_PRUNING_DELAY = 10 * 60 * 1000; const FIRST_PRUNING_DELAY = 10 * 60 * 1000;
const QUEUED_OPERATION_TTL = 3 * 24 * 60 * 60 * 1000; const QUEUED_OPERATION_TTL = 3 * 24 * 60 * 60 * 1000;
▲ Show 20 Lines • Show All 239 Lines • ▼ Show 20 Lines for (const threadID in queuedMembershipOperations) {
.get(threadID) .get(threadID)
?.delete(member.id), ?.delete(member.id),
}, },
]); ]);
} }
} }
}, [dispatch, enqueue, queuedMembershipOperations, threadInfos]); }, [dispatch, enqueue, queuedMembershipOperations, threadInfos]);
const shimmedOperations = useSelector(
state => state.queuedDMOperations.shimmedOperations,
);
const unshimmingStarted = React.useRef<boolean>(false);
React.useEffect(() => {
if (unshimmingStarted.current || shimmedOperations.length === 0) {
return;
}
unshimmingStarted.current = true;
kamilUnsubmitted
Not Done
Inline Actions

This is never set false to make sure we attempt unshimming only once after app start? If so, might be good to add code coment

kamil: This is never set false to make sure we attempt unshimming only once after app start? If so…
tomekAuthorUnsubmitted
Done
Inline Actions

This is never set false to make sure we attempt unshimming only once after app start?

Yes, exactly

tomek: > This is never set false to make sure we attempt unshimming only once after app start? Yes…
const operations: Array<QueueItem> = [];
for (const operation of shimmedOperations) {
operations.push(
{
type: 'operation',
operation: operation.operation,
},
{
type: 'action',
action: {
type: reportUnshimmingOperationCompletedActionType,
payload: {
id: operation.id,
},
},
},
);
kamilUnsubmitted
Not Done
Inline Actions

My memory about queue model we have is rusty, but I assume it is safe to perform operation and separately dispatch action? (I am talking about case when e.g. operation could not be processed)

kamil: My memory about queue model we have is rusty, but I assume it is safe to perform operation and…
tomekAuthorUnsubmitted
Done
Inline Actions

Yes, this is the same pattern we're using in other places. Both operation and removing it could fail, but there isn't too much we can do in such cases.

tomek: Yes, this is the same pattern we're using in other places. Both operation and removing it could…
}
kamilUnsubmitted
Not Done
Inline Actions

I would use flatMap() but this is fine too

kamil: I would use `flatMap()` but this is fine too
tomekAuthorUnsubmitted
Done
Inline Actions

I think we should replace this code a bit to follow patterns from this file.

tomek: I think we should replace this code a bit to follow patterns from this file.
enqueue(operations);
}, [enqueue, shimmedOperations]);
return null; return null;
} }
export { DMOpsQueueHandler }; export { DMOpsQueueHandler };
View Options

lib/types/dm-ops.js

Show First 20 Lines • Show All 585 Lines • ▼ Show 20 Lines
}; };
export const saveUnsupportedOperationActionType = 'SAVE_UNSUPPORTED_OPERATION'; export const saveUnsupportedOperationActionType = 'SAVE_UNSUPPORTED_OPERATION';
export type SaveUnsupportedOperationPayload = { export type SaveUnsupportedOperationPayload = {
+operation: DMOperation, +operation: DMOperation,
+id: string, +id: string,
}; };
export const reportUnshimmingOperationCompletedActionType =
'REPORT_UNSHIMMING_OPERATION_COMPLETED';
export type ReportUnshimmingOperationCompletedPayload = {
+id: string,
};
export type OperationsQueue = $ReadOnlyArray<{ export type OperationsQueue = $ReadOnlyArray<{
+operation: DMOperation, +operation: DMOperation,
+timestamp: number, +timestamp: number,
}>; }>;
export type QueuedDMOperations = { export type QueuedDMOperations = {
+threadQueue: { +threadQueue: {
+[threadID: string]: OperationsQueue, +[threadID: string]: OperationsQueue,
}, },
+messageQueue: { +messageQueue: {
+[messageID: string]: OperationsQueue, +[messageID: string]: OperationsQueue,
}, },
+entryQueue: { +entryQueue: {
+[entryID: string]: OperationsQueue, +[entryID: string]: OperationsQueue,
}, },
+membershipQueue: { +membershipQueue: {
+[threadID: string]: { +[threadID: string]: {
+[memberID: string]: OperationsQueue, +[memberID: string]: OperationsQueue,
}, },
}, },
+shimmedOperations: $ReadOnlyArray<SaveUnsupportedOperationPayload>,
}; };
View Options

lib/types/redux-types.js

Show First 20 Lines • Show All 48 Lines • ▼ Show 20 Linesimport type {
QueuedDMOperations, QueuedDMOperations,
QueueDMOpsPayload, QueueDMOpsPayload,
PruneDMOpsQueuePayload, PruneDMOpsQueuePayload,
ClearQueuedThreadDMOpsPayload, ClearQueuedThreadDMOpsPayload,
ClearQueuedMessageDMOpsPayload, ClearQueuedMessageDMOpsPayload,
ClearQueuedEntryDMOpsPayload, ClearQueuedEntryDMOpsPayload,
ClearQueuedMembershipDMOpsPayload, ClearQueuedMembershipDMOpsPayload,
SaveUnsupportedOperationPayload, SaveUnsupportedOperationPayload,
ReportUnshimmingOperationCompletedPayload,
} from './dm-ops.js'; } from './dm-ops.js';
import type { DraftStore } from './draft-types.js'; import type { DraftStore } from './draft-types.js';
import type { EnabledApps, SupportedApps } from './enabled-apps.js'; import type { EnabledApps, SupportedApps } from './enabled-apps.js';
import type { import type {
RawEntryInfo, RawEntryInfo,
EntryStore, EntryStore,
SaveEntryPayload, SaveEntryPayload,
CreateEntryPayload, CreateEntryPayload,
▲ Show 20 Lines • Show All 1,616 Lines • ▼ Show 20 Lines
| { | {
+type: 'RESTORE_USER_SUCCESS', +type: 'RESTORE_USER_SUCCESS',
+payload: RestoreUserResult, +payload: RestoreUserResult,
+loadingInfo: LoadingInfo, +loadingInfo: LoadingInfo,
} }
| { | {
+type: 'SAVE_UNSUPPORTED_OPERATION', +type: 'SAVE_UNSUPPORTED_OPERATION',
+payload: SaveUnsupportedOperationPayload, +payload: SaveUnsupportedOperationPayload,
}
| {
+type: 'REPORT_UNSHIMMING_OPERATION_COMPLETED',
+payload: ReportUnshimmingOperationCompletedPayload,
}, },
}>; }>;
export type ActionPayload = ?(Object | Array<*> | $ReadOnlyArray<*> | string); export type ActionPayload = ?(Object | Array<*> | $ReadOnlyArray<*> | string);
export type DispatchSource = 'tunnelbroker' | 'tab-sync'; export type DispatchSource = 'tunnelbroker' | 'tab-sync';
// Data added when dispatching action as a result of message received // Data added when dispatching action as a result of message received
// from other peer. It is used to send processing confirmation. // from other peer. It is used to send processing confirmation.
export type InboundActionMetadata = { export type InboundActionMetadata = {
Show All 27 Lines
View Options

lib/utils/reducers-utils.test.js

Loading...
View Options

native/redux/default-state.js

Loading...
View Options

native/redux/persist.js

Loading...
View Options

web/redux/default-state.js

Loading...
View Options

web/redux/persist-constants.js

Loading...
View Options

web/redux/persist.js

Loading...