statement-store: source-aware local topic set in ExplicitAffinity#12308
statement-store: source-aware local topic set in ExplicitAffinity#12308AndreiEres wants to merge 5 commits into
Conversation
| // along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
|
||
| //! Explicit topic affinity for the v2 DHT gossip path. | ||
| //! |
There was a problem hiding this comment.
This hunk is for AI, not humans.
| /// tracks how many holders of that category want the topic. | ||
| #[allow(dead_code)] | ||
| #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] | ||
| pub(crate) enum AffinitySource { |
There was a problem hiding this comment.
Why there is no DHT based affinity?
There was a problem hiding this comment.
IIUC, it's not a responsibility of this module. I beleive we can't add topic based on DHT affinity as we don't determine the topics for that, we compute the distance.
There was a problem hiding this comment.
well yes, but then would be still nice to have a list of topics that we store in general for future peers steering, and the fastest way is basically add every topic that we observe and DHT affinate to to some list as well. so why it can't be this list?
There was a problem hiding this comment.
Created an issues to think about it while doing peer stearing
| //! drops. Expose `topics()` to read the current set. Source-keying can't be retrofitted cheaply, | ||
| //! so it comes first; the enum is the extension point for future sources; `topics()` also feeds | ||
| //! the peers-topology module (#11933). | ||
| //! 4. [x] **Configured source.** Read topics from CLI at construction and `add_topics(Configured, |
There was a problem hiding this comment.
Why is this marked as done ?
There was a problem hiding this comment.
Was done in the different PR already :-)
| pub(crate) fn add_topics(&mut self, source: AffinitySource, topics: &[Topic]) { | ||
| log::trace!(target: LOG_TARGET, "explicit_affinity: add_topics {} from {source:?}", topics.len()); | ||
| for &topic in topics { | ||
| *self.local.entry(topic).or_default().entry(source).or_insert(0) += 1; |
There was a problem hiding this comment.
maybe follow the same as in func remove_topics?
let count = self.local.entry(topic).or_default().entry(source).or_insert(0);
*count = count.saturating_add(1);
| } | ||
|
|
||
| #[test] | ||
| fn add_then_remove_same_source() { |
There was a problem hiding this comment.
This test looks redundant with the final remove/assert steps in the two refcount tests below. Could we drop it and keep only the tests that cover distinct invariants
AndreiEres
force-pushed
the
andrei/explicit-affinity-local-topic-set
branch
from
fb39d1d to
7bd9815
Compare
…ity-local-topic-set
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
4 participants
Part of #11934: a source-aware local topic set in ExplicitAffinity. Replaces the stub add_topics/remove_topics with a per-topic, per-source reference-count map; a topic stays present until its last source drops. Dead code behind v2dht_enabled (off).