Fixing the issue with duplicate transaction synchronization processes #2324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xgreenx
added
no changelog
xgreenx
added
no changelog
MitchTurner
reviewed
Oct 9, 2024
|
|
||
| impl<Output> ExecutionTime<Output> { | ||
| /// Unwraps the future output and returns the execution report. | ||
| pub fn unwrap(self, metric: &FuturesMetrics) -> Output { |
There was a problem hiding this comment.
Can we name this something else? Like record_to_metrics or something like that.
The unwrap actually threw me off lol. I was reading it as unwrap_or for some Result and that made me confused about all the types.
AurelienFT
previously approved these changes
Oct 9, 2024
AurelienFT
previously approved these changes
Oct 10, 2024
AurelienFT
previously approved these changes
Oct 11, 2024
AurelienFT
approved these changes
Oct 11, 2024
rymnc
reviewed
Oct 11, 2024
|
|
||
| ### Changed | ||
|
|
||
| #### Breaking |
There was a problem hiding this comment.
all of these entries(added/fixed/changes/breaking) have different sizes
is that intentional? 🤔
Testnet already uses this image and values as expected=) |
rymnc
previously approved these changes
Oct 11, 2024
rymnc
approved these changes
Oct 11, 2024
Dentosal
approved these changes
Oct 11, 2024
| impl<Output> ExecutionTime<Output> { | ||
| /// Extracts the future output and records the execution report into the metrics. | ||
| pub fn extract(self, metric: &FuturesMetrics) -> Output { | ||
| // TODO: Use `u128` when `AtomicU128` is stable. |
There was a problem hiding this comment.
nit: we shoould probably have an issue for this
There was a problem hiding this comment.
Also it might be wise to use something like https://docs.rs/portable-atomic for now
Merged
rafal-ch
added a commit
that referenced
this pull request
Oct 11, 2024
## Version v0.39.0 ### Added - [2324](#2324): Added metrics for sync, async processor and for all GraphQL queries. - [2320](#2320): Added new CLI flag `graphql-max-resolver-recursive-depth` to limit recursion within resolver. The default value it "1". ## Fixed - [2320](#2320): Prevent `/health` and `/v1/health` from being throttled by the concurrency limiter. - [2322](#2322): Set the salt of genesis contracts to zero on execution. - [2324](#2324): Ignore peer if we already are syncing transactions from it. #### Breaking - [2320](#2330): Reject queries that are recursive during the resolution of the query. ### Changed #### Breaking - [2311](#2311): Changed the text of the error returned by the executor if gas overflows. ## What's Changed * chore(executor): instrument errors to be more meaningful by @rymnc in #2311 * fix(dummy_da_block_costs): remove dependency on polling_interval and use channel instead by @rymnc in #2314 * fix(txpool): Error in GossipsubMessageAcceptance variant docstrings by @netrome in #2316 * refactor: Eager returns in txpool_v2::service::Task::run by @netrome in #2325 * fix(api_service): exclude health checks from throttling with ConcurrencyLimitLayer by @rymnc in #2320 * Remove the `normalize_rewards_and_costs()` function by @rafal-ch in #2293 * fix(genesis): set salt of contract on execution of genesis state configuration by @rymnc in #2322 * Fixing the issue with duplicate transaction synchronization processes by @xgreenx in #2324 * Reject queries that are recursive during the resolution by @xgreenx in #2330 **Full Changelog**: v0.38.0...v0.39.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ignore peer if we already are syncing transactions from it.
Added metrics for sync and async processor.
Added metrics for all GraphQL queries.
Checklist
Before requesting review