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.

Sign up for GitHub

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

Jump to bottom

feat: update store client Query API for autosharding #885

Merged
merged 5 commits into from
Nov 13, 2023
Merged

feat: update store client Query API for autosharding #885

merged 5 commits into from
Nov 13, 2023

Conversation

chaitanyaprem
Copy link
Collaborator

@chaitanyaprem chaitanyaprem commented Nov 9, 2023 •
edited
Loading

Description

Store API changes to support auto-sharding as per #786

Changes

  • Updated store API to support queries based on contentTopics where peer selection is done based on (autosharding)
  • Update peer selection to support finding peers supporting multiple pubsubTopics
  • Cleanup Store REST API

Tests

@chaitanyaprem chaitanyaprem requested a review from SionoiS November 9, 2023 09:48
@chaitanyaprem chaitanyaprem changed the title feat: autoshard store api feat: autoshard store api updates Nov 9, 2023
@chaitanyaprem chaitanyaprem mentioned this pull request Nov 9, 2023
Closed
@status-im-auto
Copy link

status-im-auto commented Nov 9, 2023 •
edited
Loading

Jenkins Builds

Click to see older builds (42)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ cf65195 #1 2023-11-09 09:50:06 ~1 min tests 📄log
✖️ cf65195 #1 2023-11-09 09:50:08 ~1 min tests 📄log
✔️ cf65195 #1 2023-11-09 09:50:40 ~2 min nix-flake 📄log
✔️ cf65195 #1 2023-11-09 09:51:06 ~2 min linux 📦deb
✔️ cf65195 #1 2023-11-09 09:52:41 ~4 min android 📦tgz
✔️ cf65195 #1 2023-11-09 09:53:45 ~5 min ios 📦tgz
✖️ ab8cce6 #2 2023-11-10 05:31:37 ~24 sec tests 📄log
✔️ ab8cce6 #2 2023-11-10 05:32:21 ~1 min linux 📦deb
✖️ ab8cce6 #2 2023-11-10 05:32:45 ~1 min tests 📄log
✔️ ab8cce6 #2 2023-11-10 05:33:01 ~1 min nix-flake 📄log
✔️ ab8cce6 #2 2023-11-10 05:35:27 ~4 min android 📦tgz
✔️ ab8cce6 #2 2023-11-10 05:35:58 ~4 min ios 📦tgz
✔️ 5c37e53 #3 2023-11-10 09:09:53 ~1 min linux 📦deb
✔️ 5c37e53 #3 2023-11-10 09:10:37 ~1 min nix-flake 📄log
✔️ 5c37e53 #3 2023-11-10 09:11:37 ~2 min tests 📄log
✔️ 5c37e53 #3 2023-11-10 09:12:10 ~3 min tests 📄log
✔️ 5c37e53 #3 2023-11-10 09:12:30 ~3 min android 📦tgz
✔️ 5c37e53 #3 2023-11-10 09:33:48 ~25 min ios 📦tgz
✔️ 7d12cd0 #4 2023-11-10 10:13:40 ~1 min linux 📦deb
✔️ 7d12cd0 #4 2023-11-10 10:14:22 ~1 min nix-flake 📄log
✔️ 7d12cd0 #4 2023-11-10 10:15:14 ~2 min tests 📄log
✔️ 7d12cd0 #4 2023-11-10 10:16:01 ~3 min android 📦tgz
✔️ 7d12cd0 #4 2023-11-10 10:16:15 ~3 min tests 📄log
✔️ 7d12cd0 #4 2023-11-10 10:29:09 ~16 min ios 📦tgz
✔️ 064af69 #5 2023-11-10 11:07:22 ~1 min linux 📦deb
✔️ 064af69 #5 2023-11-10 11:08:10 ~1 min nix-flake 📄log
✔️ 064af69 #5 2023-11-10 11:08:39 ~2 min tests 📄log
✔️ 064af69 #5 2023-11-10 11:08:50 ~2 min tests 📄log
✔️ 064af69 #5 2023-11-10 11:09:32 ~3 min ios 📦tgz
✔️ 064af69 #5 2023-11-10 11:09:49 ~3 min android 📦tgz
✔️ b5df584 #6 2023-11-10 12:53:14 ~1 min linux 📦deb
✔️ b5df584 #6 2023-11-10 12:53:44 ~1 min nix-flake 📄log
✔️ b5df584 #6 2023-11-10 12:54:27 ~2 min tests 📄log
✔️ b5df584 #6 2023-11-10 12:55:33 ~3 min android 📦tgz
✔️ b5df584 #6 2023-11-10 12:55:36 ~3 min ios 📦tgz
✔️ b5df584 #6 2023-11-10 12:55:45 ~3 min tests 📄log
✔️ 555f06c #7 2023-11-12 03:34:23 ~1 min linux 📦deb
✔️ 555f06c #7 2023-11-12 03:35:02 ~1 min nix-flake 📄log
✔️ 555f06c #7 2023-11-12 03:35:29 ~2 min tests 📄log
✔️ 555f06c #7 2023-11-12 03:36:36 ~3 min tests 📄log
✔️ 555f06c #7 2023-11-12 03:37:06 ~3 min ios 📦tgz
✔️ 555f06c #7 2023-11-12 03:37:07 ~4 min android 📦tgz
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 9764801 #8 2023-11-13 06:38:16 ~1 min linux 📦deb
✔️ 9764801 #8 2023-11-13 06:38:55 ~1 min nix-flake 📄log
✔️ 9764801 #8 2023-11-13 06:39:21 ~2 min tests 📄log
✔️ 9764801 #8 2023-11-13 06:39:30 ~2 min tests 📄log
✔️ 9764801 #8 2023-11-13 06:40:41 ~3 min android 📦tgz
✔️ 9764801 #8 2023-11-13 06:41:03 ~4 min ios 📦tgz
✔️ 12575ce #9 2023-11-13 12:49:46 ~1 min linux 📦deb
✔️ 12575ce #9 2023-11-13 12:50:30 ~1 min nix-flake 📄log
✔️ 12575ce #9 2023-11-13 12:50:58 ~2 min tests 📄log
✔️ 12575ce #9 2023-11-13 12:52:31 ~3 min android 📦tgz
✔️ 12575ce #9 2023-11-13 12:52:39 ~4 min ios 📦tgz
✔️ 12575ce #9 2023-11-13 12:53:15 ~4 min tests 📄log

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 9, 2023
View reviewed changes
cmd/waku/server/rest/store.go Fixed Show fixed Hide fixed
cmd/waku/server/rest/store.go Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 10, 2023
View reviewed changes
waku/v2/peermanager/peer_discovery.go Dismissed Show dismissed Hide dismissed
waku/v2/peermanager/peer_discovery.go Dismissed Show dismissed Hide dismissed
waku/v2/peermanager/peer_selection.go Dismissed Show dismissed Hide dismissed
waku/v2/peermanager/peer_selection.go Dismissed Show dismissed Hide dismissed
@chaitanyaprem chaitanyaprem changed the title feat: autoshard store api updates feat: update store client Query API for autosharding Nov 10, 2023
@chaitanyaprem chaitanyaprem marked this pull request as ready for review November 10, 2023 10:13
SionoiS
SionoiS approved these changes Nov 10, 2023
View reviewed changes
Copy link

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

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

I don't fully understand GO but LGTM.

waku/v2/peerstore/waku_peer_store.go Show resolved Hide resolved
richard-ramos
richard-ramos reviewed Nov 10, 2023
View reviewed changes
cmd/waku/server/rest/store.go Outdated Show resolved Hide resolved
waku/v2/protocol/store/waku_store_client.go Show resolved Hide resolved
waku/v2/peermanager/peer_manager.go Outdated Show resolved Hide resolved
waku/v2/peerstore/waku_peer_store.go Show resolved Hide resolved
@chaitanyaprem chaitanyaprem mentioned this pull request Nov 12, 2023
Closed
1 task
@chaitanyaprem
Copy link
Collaborator Author

@richard-ramos , looks like conventional commits check is failing for commit messages in this PR.
Can we allow for existing commits to not follow this?
I am not sure if it is a good idea to change older commit messages.

@richard-ramos
Copy link
Member

looks like conventional commits check is failing for commit messages in this PR.

The fix is very easy :)

git fetch
git rebase origin/master        # This will make the commit history lineal and get rid also of the awful merge commits

and afterwards:

git rebase -i HEAD~5

And edit lines 1,2 and 4 so they look like this:

reword a6387751 rename topic to pubsubtopic in storeQuery                         
reword d8e9ce06 draft store api changes for autosharding                          
pick 6340702f feat: update peer selection to be based on multiple pubsubTopics, update store API
reword 904647dd connect to the store peer that is added via API                   
reword 086bfac8 chore: address review comments

and then, fix the messages so they look nice and tidy!

Changing older commits might be an issue only on master because other PRs might depend on those, but in this case since this PR is not the base of any other, it should be fine

@chaitanyaprem
Copy link
Collaborator Author

looks like conventional commits check is failing for commit messages in this PR.

The fix is very easy :)

git fetch
git rebase origin/master        # This will make the commit history lineal and get rid also of the awful merge commits

and afterwards:

git rebase -i HEAD~5

And edit lines 1,2 and 4 so they look like this:

reword a6387751 rename topic to pubsubtopic in storeQuery                         
reword d8e9ce06 draft store api changes for autosharding                          
pick 6340702f feat: update peer selection to be based on multiple pubsubTopics, update store API
reword 904647dd connect to the store peer that is added via API                   
reword 086bfac8 chore: address review comments

and then, fix the messages so they look nice and tidy!

Changing older commits might be an issue only on master because other PRs might depend on those, but in this case since this PR is not the base of any other, it should be fine

Thanks, learned something new :)

@chaitanyaprem chaitanyaprem force-pushed the feat/autoshard-store-api branch from 9764801 to 12575ce Compare November 13, 2023 12:48
@chaitanyaprem chaitanyaprem merged commit a5ce5df into master Nov 13, 2023
10 of 12 checks passed
@chaitanyaprem chaitanyaprem deleted the feat/autoshard-store-api branch November 13, 2023 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SionoiS SionoiS SionoiS approved these changes

@richard-ramos richard-ramos richard-ramos approved these changes

@harsh-98 harsh-98 Awaiting requested review from harsh-98

Assignees

@chaitanyaprem chaitanyaprem

Labels
None yet
Projects
Waku
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chaitanyaprem @status-im-auto @richard-ramos @SionoiS