Skip to content

Conversation

@gusinacio
Copy link
Contributor

Closes #289

@coveralls
Copy link

coveralls commented Sep 21, 2024

Pull Request Test Coverage Report for Build 11127757675

Details

  • 155 of 162 (95.68%) changed or added relevant lines in 4 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.6%) to 68.902%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tap-agent/src/agent/sender_account.rs 29 36 80.56%
Files with Coverage Reduction New Missed Lines %
tap-agent/src/agent/sender_account.rs 1 93.8%
Totals Coverage Status
Change from base Build 10967317486: 0.6%
Covered Lines: 4347
Relevant Lines: 6309

💛 - Coveralls

@gusinacio gusinacio force-pushed the gusinacio/buffer_unaggregated_fees_tracker branch from ebeed53 to e61ad31 Compare September 30, 2024 14:19
@gusinacio gusinacio marked this pull request as ready for review September 30, 2024 14:40
SenderAllocationMessage::NewReceipt(NewReceiptNotification {
id, value: fees, ..
}) => {
SenderAllocationMessage::NewReceipt(notification) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This update mostly solves the problem but it's not 100% accurate. We have a system where we trigger a complete re-calculation of unnagregated receipts. When this happens, newer receipts that are in the queue would already be on the counter and that's why we check if id > unaggregated_fees.last_id so we don't have a double counter on that.

The problem is that current system for buffer needs them to be called via add instead of update to be added to the buffer counter. That means that information about what is in buffer is lost under higher loads.

For this to be updated accordingly, tracker would need to have an internal last_id meaning that the value is added to the buffer, but not to the total tracker.

This is not a big issue since the update is usually called in rav requests which reduces total amount, but this issue must be tracked in case problems happen.

@gusinacio gusinacio requested a review from mangas September 30, 2024 19:44
Signed-off-by: Gustavo Inacio <[email protected]>
@gusinacio gusinacio requested a review from carlosvdr October 1, 2024 14:49
Copy link
Contributor

@carlosvdr carlosvdr left a comment

Choose a reason for hiding this comment

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

lgtm

@gusinacio gusinacio merged commit 676a437 into main Oct 1, 2024
10 checks passed
@gusinacio gusinacio deleted the gusinacio/buffer_unaggregated_fees_tracker branch October 1, 2024 16:11
@github-actions github-actions bot mentioned this pull request Oct 9, 2024
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.

Track unaggregated fees inside and outside the buffer

4 participants