Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSConsistencyManager & IAM fetch read-your-write consistency implementation #1486

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

rgomezp
Copy link
Contributor

@rgomezp rgomezp commented Sep 27, 2024

Description

One Line Summary

Introduce and integrate a ConsistencyManager to manage read-your-write tokens for improved segment membership calculation.

Details

Motivation

This update introduces the ConsistencyManager to manage RYW tokens. The goal is to improve the accuracy of segment membership calculations by providing an open-ended & highly customizable blocking mechanism for operations that rely on having successfully synchronized client & server state.

For a first use-case, we want to block the fetching of IAMs until we have tokens for a user or subscription state update.

Scope

  • OSConsistencyManager implementation
  • Updates the Subscription and User backend services to return offsets needed by the OSConsistencyManager.
  • Modifies Subscription and UpdateUser operation executors to set tokens in the OSConsistencyManager.
  • Updates the OSMessagingController to delay IAM fetch until the token is available. Implements the retry logic.

Testing

Unit testing

  • New coverage for new ConsistencyManager class

Manual Testing

  • Set up iOS test app in OneSignal
  • Note the current app version (e.g. 1.0)
  • Create a segment targeting a specific version (e.g. 1.1)
  • Create an in-app using the new segment
  • Open the app (see you don't get an in-app)
  • Close the app
  • Change the version to the one in the segment (e.g. 1.1)
  • Open the app (see you get the in-app)

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

@rgomezp rgomezp changed the title Consistency manager OSConsistencyManager & IAM fetch read-your-write consistency implementation Sep 27, 2024
@rgomezp rgomezp force-pushed the consistency-manager branch 3 times, most recently from 4fbe142 to 7f07d0a Compare September 30, 2024 17:53
@rgomezp rgomezp force-pushed the consistency-manager branch 4 times, most recently from ebcda07 to 8be39ed Compare October 3, 2024 19:50
@rgomezp rgomezp force-pushed the consistency-manager branch 7 times, most recently from 6dbf0ca to d546445 Compare October 10, 2024 22:17
private override init() {}

// Function to set the token in a thread-safe manner
@objc public func setRywToken(id: String, key: OSIamFetchOffsetKey, value: String?) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only expected usage of this method is for IAM Fetch?

Copy link
Contributor Author

@rgomezp rgomezp Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire class can be used in the future for other RYW consistency projects!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I wanted to clarify as the parameter is explicitly OSIamFetchOffsetKey

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops that's a mistake

iOS_SDK/OneSignalSDK/Source/OneSignalTracker.m Outdated Show resolved Hide resolved
@rgomezp rgomezp requested a review from nan-li October 15, 2024 18:32
@rgomezp rgomezp mentioned this pull request Oct 15, 2024
18 tasks
@rgomezp rgomezp force-pushed the consistency-manager branch 2 times, most recently from d5540ae to 9c404e7 Compare October 16, 2024 19:10
@rgomezp rgomezp removed the request for review from emawby October 16, 2024 20:32
Motivation: encapsulating object to keep rywTokens & rywDelays together

Override equality
Motivation: Manages (kafka) offsets that function as read-your-write tokens for more accurate segment membership calculation. The manager works based on conditions & offsets.

Offsets are stored in a nested map indexed by a unique id (e.g. `onesignalId`) and offset key (e.g. `USER_UPDATE`).

This allows us to track offsets on a per-user basis (e.g. handle switching users).

Conditions work by creating a blocking mechanism with customizable offset retrieval until a pre-defined condition is met (e.g. at least two specific offsets are available).

OSCondition interface: allows extensibility for future applications to control offset blocking mechanism in consistency use-cases.
Motivation: custom condition to block offset retrieval until condition is met: user & subscription offsets are both available or just the user offset is available. This is because we always make a user update call (update the session count) but not always for subscription.
Motivation: commit new file references
…ager`

Motivation: move into `OSSessionManager` to be accessible from other modules
Motivation: we need to pass the offset, session duration, & retry count to the GET IAM request.
Motivation: retry logic should work as follows:
1. initial request with offset --> failure
2. get retry limit & retry after from response & start retrying
3. if hit limit, make one final request with the offset: 0

The final request tells the backend, just show me what you got.

Example: If the retry limit is 3 & we never get a successful response retrying, we should end up with 5 total requests.
Motivation: executors make the update or create requests and get back an offset which is saved in the consistency manager
Motivation: we need to flush the delta queue on start in order to immediately trigger updates that would return a RYW token so we can unblock IAM fetch as soon as possible.
Created a new target OneSignalOSCoreTests
Motivation: if we have a subscription update enqueued, we need to consider it. Otherwise we should just consider the userUpdateTokenSet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants