Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

avoid creating duplicate unbacked spans when we see extra statements#2145

Merged
rphmeier merged 1 commit intomasterfrom
rh-backing-avoid-dupicate-span
Dec 18, 2020
Merged

avoid creating duplicate unbacked spans when we see extra statements#2145
rphmeier merged 1 commit intomasterfrom
rh-backing-avoid-dupicate-span

Conversation

@rphmeier
Copy link
Copy Markdown
Contributor

@rphmeier rphmeier commented Dec 18, 2020

Receiving a CandidateBackingMessage::Second or seeing a Statement::Seconded after we'd already seen requisite votes for backing led us to create extra unbacked spans that were never cleaned up.

image

@rphmeier rphmeier added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Dec 18, 2020
@rphmeier
Copy link
Copy Markdown
Contributor Author

rphmeier commented Dec 18, 2020

honestly, this is related to a larger bug in candidate backing (hooray) where we validate the candidate a second time as well. in cases where we've seen the candidate backed and then are asked to second it by another subsystem.

@rphmeier
Copy link
Copy Markdown
Contributor Author

will merge this so we can redeploy; touches no actual logic, just logging

@rphmeier rphmeier merged commit b1a4c82 into master Dec 18, 2020
@rphmeier rphmeier deleted the rh-backing-avoid-dupicate-span branch December 18, 2020 23:53
@burdges
Copy link
Copy Markdown
Contributor

burdges commented Dec 19, 2020

I suppose a signature that seconds already references who proposed?

@rphmeier
Copy link
Copy Markdown
Contributor Author

@burdges the receipt contains the collator address and signature

rphmeier added a commit that referenced this pull request Dec 20, 2020
* add candidate hash statement circulation span

* add relay-parent to hash-span

* Some typos and misspellings in docs I found, during my studies. (#2144)

* Fix stale link to overseer docs

* Some typos and mispellings in docs/comments

I found during studying how Polkadot works.

* Rococo V1 (#2141)

* Update to latest master and use 30 minutes sessions

* add bootnodes to chainspec

* Update Substrate

* Update chain-spec

* Update Cargo.lock

* GENESIS

* Change session length to one hour

* Bump spec_version to not fuck anything up ;)

Co-authored-by: Erin Grasmick <erin@parity.io>

* avoid creating duplicate unbacked spans when we see extra statements (#2145)

* improve jaeger spans for statement distribution

* tweak and add failing test for repropagation

* make a change that gets the test passing

* guide: clarify

* remove semicolon

Co-authored-by: Robert Klotzner <eskimor@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Erin Grasmick <erin@parity.io>
@coriolinus
Copy link
Copy Markdown
Contributor

@rphmeier Just following up on

this is related to a larger bug in candidate backing

Does that larger bug have its own issue? If so, I can't seem to find it: the most recent issue created which contains the phrase "candidate backing" seems to have been from 10 days before this PR.

@rphmeier
Copy link
Copy Markdown
Contributor Author

rphmeier commented Jan 4, 2021

@coriolinus I didn't file one, no, but please go ahead if you like. It's not a terrible bug since the sanity checks before statement issuance will protect us from issuing conflicting statements. Just wasted a little bit of CPU

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants