Persistent storage for replays (liammeier/fence-1196) #271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lmeier
requested review from
tjulien,
nickpatrick,
corypisano,
jsani-radar and
KennyHuRadar
jsani-radar
reviewed
Aug 17, 2023
RadarSDK/Radar.m
Outdated
| @@ -53,6 +54,8 @@ + (void)initializeWithPublishableKey:(NSString *)publishableKey { | |||
| [RadarSettings updateSessionId]; | |||
| } | |||
|
|
|||
| [[RadarReplayBuffer sharedInstance] loadReplaysFromPersistentStore]; | |||
There was a problem hiding this comment.
don't have a strong opinion, but maybe we should load replays into memory after the call to /config is made and tracking options are updated
There was a problem hiding this comment.
Yea -- perhaps we gate this on replays: all after the config.
RadarSDK/RadarReplayBuffer.m
Outdated
| @@ -42,6 +43,10 @@ - (void)writeNewReplayToBuffer:(NSMutableDictionary *)replayParams { | |||
| // add new replay to buffer | |||
| RadarReplay *radarReplay = [[RadarReplay alloc] initWithParams:replayParams]; | |||
| [mutableReplayBuffer addObject:radarReplay]; | |||
|
|
|||
| // persist buffer | |||
There was a problem hiding this comment.
in an ideal world would this be encrypted?
tjulien
approved these changes
Aug 18, 2023
jsani-radar
reviewed
Aug 28, 2023
RadarSDK/RadarReplayBuffer.m
Outdated
| NSMutableArray<RadarReplay *> *prunedBuffer; | ||
|
|
||
| // If buffer length is above 50, remove every fifth replay from the persisted buffer | ||
| if ([mutableReplayBuffer count] > 50) { |
There was a problem hiding this comment.
hmm, wondering under what circumstances this would happen? when would /track/replay realistically return a non-200?
could be wrong but if I understand correctly, the endpoint returns a 201 and does everything async after the fanout - regardless, we should handle SQS being down gracefully
There was a problem hiding this comment.
In early replay testing, I witnessed some 413 Payload Too Large's
lmeier
force-pushed
the
liammeier/fence-1196-persistent-storage-for-replays
branch
from
6f3199a to
7865fe4
Compare
lmeier
force-pushed
the
liammeier/fence-1196-persistent-storage-for-replays
branch
from
e830e61 to
904a77e
Compare
ShiCheng-Lu
pushed a commit
that referenced
this pull request
Jun 13, 2024
* publish SNAPSHOT releases from pre-releases not from pushing to develop (#259) * don't publish pre-releases to maven as a release * publish SNAPSHOTs from pre-releases not from pushing to develop * MOB-57: update the README with more detail on example app (#260) Co-authored-by: Jason R Tibbetts <[email protected]> Co-authored-by: Nick Patrick <[email protected]> Co-authored-by: Tim Julien <[email protected]> * Surface center, radius, and location for geofences & places (#267) * Surface center, radius, and location for geofences & places * rely on geofence's geometry for center and radius, add test * RAD-5747 Anonymous config call in `trackOnce` (#266) * add config call for anonymous before track * add boolean for track usage * usage enum not bool * resume usage * move config track usage call later in the call stack * move config track usage call later in the call stack * use anon val * bump version * 3.5.11 version bump --------- Co-authored-by: Jason R Tibbetts <[email protected]> Co-authored-by: Nick Patrick <[email protected]> Co-authored-by: Tim Julien <[email protected]> * Include approachingThreshold in tripOptions (#261) * Include approachingThreshold in trip options helper functions * Only include in dictionary if > 0, add test * Offline replay (#270) * First part of offline replay * Fix kotlin errors * Fix params for replays * Extend timeout * Fix unallocated val bug * Fix nowMS bug * Fix connectTimeout numbers * Bump version to 3.5.12-beta.2 to cut a prerelease * Bump nexus client timeout * Fix bugs in buffer logic * bump version to 3.5.11-beta.6 * dont fire event listeners for custom events (#263) * publish beta's the same way we publish final releases (#273) * publish beta's the same way we publish final releases * Delete radar-snapshot-action.yml * Update build.gradle * Always clear last location if successful track (#272) * Always clear last location if succesful track * Always use System.currentTimeMillis() * Always clear ReplayBuffer upon successful request --------- Co-authored-by: Tim Julien <[email protected]> * version bump * set replay = NONE for continuous * not beta anymore --------- Co-authored-by: Travis Derouin <[email protected]> Co-authored-by: Jason R Tibbetts <[email protected]> Co-authored-by: Nick Patrick <[email protected]> Co-authored-by: Liam Meier <[email protected]> Co-authored-by: David Goodfellow <[email protected]> Co-authored-by: jsani-radar <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We utilize
NSUserDefaultsto persist replays every time we write to the replay buffer, and load the replay buffer into memory uponinitialize.Local testing is proceeding well. I'll implement in android, and cut a Waypoint with Kenny this afternoon for larger testing.