Consolidate speedtest average timestamps s3/iceberg - #1246
Merged
Conversation
Iceberg was using Utc::now() when the speedtest average was constructed before being inserted. S3 was using Utc::now() at the time the record was written to the proto file. This makes it impossible to correctly correlate 2 speedtest averages between the 2 data sources. A side-effect of this change is that speedtest averages timestamps are truncated to the nearest second to match what is stored in the proto file. Both sources will be using the same datetime value and encoded to seconds so a proper correlation can be done between sources.
michaeldjeffrey
marked this pull request as ready for review
September 3, 2026 16:47
bbalser
approved these changes
Sep 4, 2026
michaeldjeffrey
added a commit
that referenced
this pull request
Sep 8, 2026
Two conflicts, both from HIP-150 (#1245) and the speedtest-average timestamp consolidation (#1246) landing first. - speedtests_average.rs: keep #1246's `DateTime`/`SubsecRound` imports, drop `speedtests::self` — this branch removed `SpeedtestAverages`, its only user. - rewarder_sp_rewards.rs: keep HIP-150's suspended-pool assertions, but call `reward_service_providers` with the now-required iceberg writers instead of `None`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
michaeldjeffrey
added a commit
that referenced
this pull request
Sep 8, 2026
Two conflicts, both from HIP-150 (#1245) and the speedtest-average timestamp consolidation (#1246) landing first. - speedtests_average.rs: keep #1246's `DateTime`/`SubsecRound` imports, drop `speedtests::self` — this branch removed `SpeedtestAverages`, its only user. - rewarder_sp_rewards.rs: keep HIP-150's suspended-pool assertions, but call `reward_service_providers` with the now-required iceberg writers instead of `None`.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Iceberg was using Utc::now() when the speedtest average was constructed before being inserted.
S3 was using Utc::now() at the time the record was written to the proto file.
This makes it impossible to correctly correlate 2 speedtest averages between the 2 data sources.
A side-effect of this change is that speedtest averages timestamps are truncated to the nearest second to match what is stored in the proto file.
Both sources will be using the same datetime value and encoded to seconds so a proper correlation can be done between sources.