Skip to content

/ sync

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

Introduce device id v2 #362

Merged
merged 5 commits into from Dec 23, 2019
Merged

Introduce device id v2 #362

merged 5 commits into from Dec 23, 2019

Conversation

@darkdh
Copy link
Member

darkdh commented Dec 6, 2019 •
edited

fix #333

This PR introduce device id v2 which is a 3 random bytes hex encoded string.
We also keep device id for backward compatibility.
Migration from existing device would be:

  1. There will be extra SAVE_INIT_DATA for updating device id v2
  2. Create new SQS queues using device id v2 and subscribe to old SQS queues created by device id
  3. We will fetch from both queues for 24 hours and then delete the old queues
darkdh added 3 commits Nov 13, 2019
@darkdh
uuid init
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
4e5ed88
@darkdh
deviceIdV2 and SQS queue migration
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
9e96726
@darkdh
Emit SAVE_INIT_DATA for existing devices to update deviceIdV2
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
77af179
@darkdh darkdh force-pushed the dup-device-id branch 3 times, most recently from 109cdd6 to 275c5f2 Dec 9, 2019
@darkdh
fix legacy device id reset issue
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
0387b4e 
fix test

add comments
@darkdh darkdh force-pushed the dup-device-id branch from 275c5f2 to 0387b4e Dec 9, 2019
@darkdh darkdh self-assigned this Dec 9, 2019
@darkdh
Add device id v2 test
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
f6a25c1
@darkdh darkdh force-pushed the dup-device-id branch from c83f8fd to f6a25c1 Dec 10, 2019
@darkdh darkdh marked this pull request as ready for review Dec 10, 2019
@darkdh
darkdh reviewed Dec 12, 2019
View changes
client/requestUtil.js
return s3Helper.listNotifications(
this.sqs, notificationParams, category, prefix).then((values) => {
if (this.shouldRetireOldSQSQueue(parseInt(values.createdTimeStamp))) {
return this.deleteSQSQueue(this.oldSQSUrlByCat[category]).then(() => {

This comment has been minimized.

Sign in to view
Copy link
@darkdh

darkdh Dec 12, 2019

Author Member

@AlexeyBarabash reminded me that when device id is duplicated, if there is an old Brave which doesn't contain the fix, its SQS queue will be unavailable until next relaunch because upgraded Brave deletes it after 24 hours

This comment has been minimized.

Sign in to view
Copy link
@AlexeyBarabash

AlexeyBarabash Dec 12, 2019

Contributor

I was confused and I thought this.deleteSQSQueue can delete queues belonging to other devices. And like @darkdh mentioned that only can happen if device id was duplicated. Which is a separate case.

This comment has been minimized.

Sign in to view
Copy link
@darkdh

darkdh Dec 13, 2019

Author Member

Non upgraded device will get reset anyway, so duplicate case won’t be an issue
@darkdh darkdh mentioned this pull request Dec 12, 2019
Closed
@SergeyZhukovsky
SergeyZhukovsky approved these changes Dec 12, 2019
View changes
Copy link
Member

SergeyZhukovsky left a comment

++
@AlexeyBarabash
Copy link
Contributor

AlexeyBarabash commented Dec 12, 2019

Minor notice
4e5ed88 may be squashed before merge, unless there is a hope it will be helpful somehow.

All the rest looks good.
@AlexeyBarabash
AlexeyBarabash approved these changes Dec 15, 2019
View changes
Copy link
Contributor

AlexeyBarabash left a comment

++
@bridiver
bridiver approved these changes Dec 23, 2019
View changes
@bridiver bridiver merged commit aed5381 into staging Dec 23, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
bridiver added a commit to brave/brave-core that referenced this pull request Dec 26, 2019
@bridiver
bump deps for brave/sync#362
f5ba9dd
darkdh added a commit that referenced this pull request Dec 28, 2019
@bridiver @darkdh
Merge pull request #362 from brave/dup-device-id
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
Loading status checks…
17aca21 
Introduce device id v2
@darkdh darkdh mentioned this pull request Dec 28, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bridiver bridiver

@SergeyZhukovsky SergeyZhukovsky

@AlexeyBarabash AlexeyBarabash

Assignees

@darkdh darkdh

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Two different devices in same chain with same id
4 participants
@darkdh @AlexeyBarabash @bridiver @SergeyZhukovsky
You can’t perform that action at this time.