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

feat: replace DefaultPubsubTopic by Shard 32 #4161

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Conversation

richard-ramos
Copy link
Member

This PR removes the usage of default pubsub topic. It is not possible to mix named and static sharding, and the default pubsub topic is a named shard. With this PR, I'm using shard rs/16/32 as the default shard for all non-community messages.

This is somewhat problematic because it's a breaking change and compatibility between diff versions would be lost? not sure how to solve this. Perhaps the status.prod fleet should also listen to shard rs/16/32 before this PR is merged?

@richard-ramos
Copy link
Member Author

For some reason make vendor included A LOT of files

@status-im-auto
Copy link
Member

status-im-auto commented Oct 16, 2023

Jenkins Builds

Click to see older builds (40)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ dc97f40 #1 2023-10-16 19:54:01 ~3 min linux 📦zip
✔️ dc97f40 #1 2023-10-16 19:56:21 ~5 min android 📦aar
✔️ dc97f40 #1 2023-10-16 19:58:40 ~7 min ios 📦zip
✖️ dc97f40 #1 2023-10-16 20:06:27 ~15 min tests 📄log
01f1afe #2 2023-10-23 21:17:35 ~20 sec android 📄log
01f1afe #2 2023-10-23 21:17:38 ~22 sec ios 📄log
01f1afe #2 2023-10-23 21:18:12 ~56 sec linux 📄log
✖️ 01f1afe #2 2023-10-23 21:19:21 ~2 min tests 📄log
✔️ 61b30c9 #3 2023-10-23 21:38:50 ~3 min linux 📦zip
✔️ 61b30c9 #3 2023-10-23 21:39:05 ~3 min ios 📦zip
✔️ 61b30c9 #3 2023-10-23 21:39:56 ~4 min android 📦aar
✖️ 61b30c9 #3 2023-10-23 21:45:24 ~10 min tests 📄log
142cf64 #4 2023-11-01 15:08:37 ~20 sec ios 📄log
142cf64 #4 2023-11-01 15:09:19 ~1 min linux 📄log
142cf64 #4 2023-11-01 15:09:44 ~1 min android 📄log
✖️ 142cf64 #4 2023-11-01 15:10:27 ~2 min tests 📄log
b289d13 #5 2023-11-01 15:59:48 ~1 min linux 📄log
b289d13 #5 2023-11-01 15:59:57 ~1 min ios 📄log
✖️ b289d13 #5 2023-11-01 16:00:40 ~1 min tests 📄log
b289d13 #5 2023-11-01 16:01:01 ~2 min android 📄log
96b6588 #6 2023-11-01 16:01:59 ~15 sec linux 📄log
96b6588 #6 2023-11-01 16:02:01 ~21 sec android 📄log
96b6588 #6 2023-11-01 16:02:21 ~41 sec ios 📄log
✖️ 96b6588 #6 2023-11-01 16:02:36 ~52 sec tests 📄log
✔️ e698160 #7 2023-11-01 17:52:37 ~2 min linux 📦zip
✔️ e698160 #7 2023-11-01 17:53:49 ~3 min ios 📦zip
✔️ e698160 #7 2023-11-01 17:54:38 ~4 min android 📦aar
✖️ e698160 #7 2023-11-01 18:07:44 ~17 min tests 📄log
✔️ 67beed1 #8 2023-11-03 16:22:01 ~3 min linux 📦zip
✔️ 67beed1 #8 2023-11-03 16:23:40 ~4 min ios 📦zip
✔️ 67beed1 #8 2023-11-03 16:24:16 ~5 min android 📦aar
✖️ 67beed1 #8 2023-11-03 16:32:31 ~13 min tests 📄log
57c5e18 #9 2023-11-09 18:11:40 ~25 sec android 📄log
57c5e18 #9 2023-11-09 18:11:40 ~21 sec ios 📄log
57c5e18 #9 2023-11-09 18:12:15 ~57 sec linux 📄log
✖️ 57c5e18 #9 2023-11-09 18:13:27 ~2 min tests 📄log
✔️ 3446216 #10 2023-11-09 18:26:48 ~2 min linux 📦zip
✔️ 3446216 #10 2023-11-09 18:27:43 ~3 min ios 📦zip
✔️ 3446216 #10 2023-11-09 18:28:52 ~4 min android 📦aar
✖️ 3446216 #10 2023-11-09 18:40:44 ~16 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ c5ecc5c #11 2023-11-09 18:28:18 ~1 min linux 📦zip
✔️ c5ecc5c #11 2023-11-09 18:30:31 ~2 min ios 📦zip
✔️ c5ecc5c #11 2023-11-09 18:30:53 ~1 min android 📦aar
✖️ c5ecc5c #11 2023-11-09 18:57:43 ~16 min tests 📄log
✖️ c5ecc5c #12 2023-11-09 19:51:49 ~15 min tests 📄log
✖️ c5ecc5c #13 2023-11-09 20:31:56 ~14 min tests 📄log
✖️ c5ecc5c #14 2023-11-09 20:50:46 ~14 min tests 📄log
✔️ d35c78d #12 2023-11-09 23:17:54 ~1 min linux 📦zip
✔️ d35c78d #12 2023-11-09 23:18:30 ~1 min android 📦aar
✔️ d35c78d #12 2023-11-09 23:19:52 ~3 min ios 📦zip
✖️ d35c78d #15 2023-11-09 23:44:52 ~28 min tests 📄log
✔️ d35c78d #16 2023-11-10 00:24:24 ~31 min tests 📄log

Copy link
Member

@vitvly vitvly left a comment

Choose a reason for hiding this comment

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

Went through, all good to me!

@vitvly
Copy link
Member

vitvly commented Oct 18, 2023

Question: in this PR i've noticed lines like community.Shard().TransportShard(), that do not check for nil return value of Shard(). Which contrasts with some other places in code, like SaveCommunity() or MarshalPublicAPIJSON() in protocol/communities package, that do this check.

This func will only return nil if Community.config is nil - but when can this happen?

@richard-ramos
Copy link
Member Author

This func will only return nil if Community.config is nil - but when can this happen?

I'm not sure when this specific condition will this happen, tbh, but something that can indeed happen is that o.config.Shard is nil which happens when no shard has been assigned to a community. Since this is such a common scenario, in TransportShard i do check if the shard is nil. That way I don't need to do a comparison for nil whenever .Shard() is used, since TransportShard will automatically know what to do

@richard-ramos
Copy link
Member Author

@cammellos, @vitvly
I did try to get around the backward compatibility issue with having some hardcoded nodes and it is problematic because even if we add some hardcoded peers, the fact that the messages get published on separate pubsub topics means that every message would have to get published on both the default pubsub topic, and in a static shard, meaning that there wouldn't be any bandwidth improvements. If anything, the bandwidth usage would increase even more due to duplicated messages being posted on separate pubsub topics.

Also the bootnodes currently cant subscribe to both named and static shards at same time, meaning that if we want to have harcoded nodes just to support backward compatibility, we can't reuse those from shards.test fleet and would require a separate fleet of nodes with a subscription to the default pubsub topic

@richard-ramos richard-ramos merged commit 2c954d4 into develop Nov 10, 2023
7 checks passed
@richard-ramos richard-ramos deleted the shard/32 branch November 10, 2023 00:29
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.

None yet

3 participants