Skip to content

Subscriberdb_cache service that monitors/updates digests on all existing networks#7472

Merged
hcgatewood merged 4 commits intomagma:masterfrom
wangyyt1013:subscriberdb_cache_app
Jun 24, 2021
Merged

Subscriberdb_cache service that monitors/updates digests on all existing networks#7472
hcgatewood merged 4 commits intomagma:masterfrom
wangyyt1013:subscriberdb_cache_app

Conversation

@wangyyt1013
Copy link
Copy Markdown
Member

@wangyyt1013 wangyyt1013 commented Jun 10, 2021 •
edited
Loading

Signed-off-by: Yuanyuting Wang [email protected]

Summary

  1. Create DigestLookup sql store that keeps a list (updated every <configurable_interval>) of deterministic digests for all existing networks
  2. Create subscriberdb_cache service that continuously monitors and updates the digest store
  3. Create helper functions in subscriberdb/digest_generator.go that is in charge of generating deterministic network configs digests, which are concatenations of the subscribers digest and apn resources digest per network

Test Plan

  1. Run all existing test cases. All should pass
  2. Added test cases for digest_lookup and utils in subscriberdb. All should pass
  3. Added test cases for subscriberdb_cache worker
  4. Manually verified on local Docker deployment

Additional Information

@wangyyt1013 wangyyt1013 requested a review from hcgatewood June 10, 2021 20:53
@pull-request-size pull-request-size bot added the size/XXL Denotes a Pull Request that changes 1000+ lines. label Jun 10, 2021
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 10, 2021

Thanks for opening a PR! 💯 Please note that all commits must be signed off. This is enforced by the DCO check.

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch 3 times, most recently from c2c115c to 51e6da8 Compare June 14, 2021 19:07
@pull-request-size pull-request-size bot added size/XL Denotes a Pull Request that changes 500-999 lines. and removed size/XXL Denotes a Pull Request that changes 1000+ lines. labels Jun 14, 2021
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from 51e6da8 to a0f883b Compare June 14, 2021 21:09
@pull-request-size pull-request-size bot added size/XXL Denotes a Pull Request that changes 1000+ lines. and removed size/XL Denotes a Pull Request that changes 500-999 lines. labels Jun 15, 2021
@wangyyt1013 wangyyt1013 self-assigned this Jun 15, 2021
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from fbec1e6 to bc5c580 Compare June 15, 2021 18:30
@wangyyt1013 wangyyt1013 marked this pull request as ready for review June 16, 2021 00:04
@wangyyt1013 wangyyt1013 requested a review from a team June 16, 2021 00:04
@wangyyt1013 wangyyt1013 changed the title [WIP] Subscriberdb_cache service that monitors/updates digests on all existing networks Subscriberdb_cache service that monitors/updates digests on all existing networks Jun 18, 2021
@magmabot magmabot added the component: agw Access gateway-related issue label Jun 18, 2021
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from 7d95978 to 41e0f41 Compare June 18, 2021 21:05
@wangyyt1013 wangyyt1013 mentioned this pull request Jun 18, 2021
Merged
hcgatewood
hcgatewood suggested changes Jun 18, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@hcgatewood hcgatewood left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Shape looks great! Looking forward to getting this landed 🙂

Comment thread lte/cloud/configs/subscriberdb_cache.yml
Comment thread orc8r/cloud/go/mproto/mproto.go Outdated
Comment thread orc8r/cloud/go/mproto/mproto.go Outdated
Comment thread orc8r/cloud/go/sqorc/table.go
Comment thread lte/cloud/go/services/subscriberdb_cache/const.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from a6d1aea to 914b9bc Compare June 19, 2021 06:29
@wangyyt1013 wangyyt1013 requested a review from hcgatewood June 21, 2021 17:07
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from 914b9bc to 37de9d2 Compare June 21, 2021 18:43
@wangyyt1013 wangyyt1013 requested review from a team and koolzz as code owners June 21, 2021 18:43
@wangyyt1013 wangyyt1013 requested review from a team June 21, 2021 18:43
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch 2 times, most recently from 66350c0 to 2f057b3 Compare June 22, 2021 16:57
hcgatewood
hcgatewood reviewed Jun 23, 2021
View reviewed changes
Comment thread lte/protos/subscriberdb.proto Outdated
Comment thread lte/protos/subscriberdb.proto Outdated
Comment thread lte/protos/subscriberdb.proto Outdated
Comment thread lte/protos/subscriberdb.proto Outdated
Comment thread lte/protos/subscriberdb.proto Outdated
Comment thread lte/cloud/go/services/subscriberdb_cache/worker.go Outdated
Comment thread lte/cloud/go/services/subscriberdb_cache/worker.go Outdated
Comment thread lte/cloud/go/services/subscriberdb_cache/worker.go Outdated
Comment thread lte/cloud/go/services/subscriberdb_cache/worker.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
hcgatewood
hcgatewood suggested changes Jun 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

First half of my comments got added to a separate review for some reason, I think. Anyways, looks good, handle the handful of comments then I think we'll be in a good spot

Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest_lookup.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from c556b0a to 8b3ff53 Compare June 23, 2021 18:02
@wangyyt1013 wangyyt1013 requested a review from hcgatewood June 23, 2021 18:02
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from 8b3ff53 to 137dc68 Compare June 23, 2021 21:43
hcgatewood
hcgatewood suggested changes Jun 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

Looks great, couple nits. Msg me when updated, I'll plan to merge the updated PR tonight 🙂

Comment thread lte/cloud/go/services/subscriberdb/digest.go
Comment thread lte/cloud/go/services/subscriberdb/digest_test.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/digest_test.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/servicers/subscriberdb_servicer.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
Comment thread lte/cloud/go/services/subscriberdb_cache/worker.go Outdated
Comment thread lte/cloud/go/services/subscriberdb_cache/worker.go Outdated
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from 137dc68 to 7c5d431 Compare June 24, 2021 02:52
@wangyyt1013 wangyyt1013 requested a review from hcgatewood June 24, 2021 03:55
hcgatewood
hcgatewood approved these changes Jun 24, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

Okay lgtm! Fix the couple nits then msg, we can merge this and the charts PR tomorrow morning, then the agw PR tomorrow afternoon

Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/digest.go Outdated
Comment thread orc8r/cloud/go/services/configurator/test_init/test_service_init.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
Comment thread lte/cloud/go/services/subscriberdb/storage/digest.go Outdated
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from f96ed62 to 89bf90a Compare June 24, 2021 13:51
@wangyyt1013
Add subscriberdb_cache service
2fa8506 
Signed-off-by: Yuanyuting Wang <[email protected]>
@wangyyt1013
address comments
62dca14 
Signed-off-by: Yuanyuting Wang <[email protected]>
@wangyyt1013
address comments
8887eb2 
Signed-off-by: Yuanyuting Wang <[email protected]>
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from 89bf90a to 8887eb2 Compare June 24, 2021 16:26
hcgatewood
hcgatewood approved these changes Jun 24, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

Okay shoot one last thing -- we need to enable the digests by default 🙂

Comment thread lte/protos/subscriberdb.proto Outdated
Comment thread lte/cloud/configs/subscriberdb.yml Outdated
@wangyyt1013 wangyyt1013 requested a review from hcgatewood June 24, 2021 17:04
@wangyyt1013
address final comments
a6dc360 
Signed-off-by: Yuanyuting Wang <[email protected]>
@wangyyt1013 wangyyt1013 force-pushed the subscriberdb_cache_app branch from e0c94a6 to a6dc360 Compare June 24, 2021 17:50
@hcgatewood hcgatewood merged commit 20eb7d7 into magma:master Jun 24, 2021
themarwhal pushed a commit that referenced this pull request Jun 24, 2021
@wangyyt1013 @themarwhal
Subscriberdb_cache service that monitors/updates digests on all exist…
62a3cdc 
…ing networks (#7472)
rmeleromira pushed a commit to rmeleromira/magma that referenced this pull request Jul 24, 2021
@wangyyt1013 @rmeleromira
Subscriberdb_cache service that monitors/updates digests on all exist…
595a969 
…ing networks (magma#7472)

Signed-off-by: Ramon Melero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@koolzz koolzz koolzz approved these changes

@themarwhal themarwhal themarwhal approved these changes

@hcgatewood hcgatewood Awaiting requested review from hcgatewood

Assignees

@wangyyt1013 wangyyt1013

Labels

apply-v1.6 backported-v1.6 component: agw Access gateway-related issue size/XXL Denotes a Pull Request that changes 1000+ lines.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@wangyyt1013 @hcgatewood @koolzz @themarwhal @magmabot