Home Original page

feat(net): fix failure to process FETCH_INV_DATA message by xxo1shine · Pull Request #5460 · tronprotocol/java-tron

Conversation

@xxo1shine

What does this PR do?
Fix failure to process FETCH_INV_DATA message, fix #5439.

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

jwrct

}
long now = System.currentTimeMillis();
invToFetch.forEach((item, time) -> {
if (time < now - MSG_CACHE_DURATION_IN_BLOCKS * BLOCK_PRODUCED_INTERVAL) {

Choose a reason for hiding this comment

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

Shouldn't MSG_CACHE_DURATION_IN_BLOCKS * BLOCK_PRODUCED_INTERVAL be replaced with TIMEOUT here?

Choose a reason for hiding this comment

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

+1

@codecov-commenter

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.45%. Comparing base (b9ab07a) to head (7e2b7fc).
⚠️ Report is 524 commits behind head on develop.

Files with missing lines Patch % Lines
...java/org/tron/core/net/service/adv/AdvService.java 50.00% 4 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files 
@@            Coverage Diff             @@
##             develop    #5460   +/-   ##
==========================================
  Coverage      61.45%   61.45%           
+ Complexity      9370     9369    -1     
==========================================
  Files            846      846           
  Lines          50298    50300    +2     
  Branches        5585     5585           
==========================================
+ Hits           30911    30913    +2     
+ Misses         16969    16968    -1     
- Partials        2418     2419    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

317787106

halibobo1205

invToFetch.remove(item);
});
peers.stream().filter(peer -> {
Long t = peer.getAdvInvReceive().getIfPresent(item);

Choose a reason for hiding this comment

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

t(L282) and time(L274) isn't that close to the same?

image image

Choose a reason for hiding this comment

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

Not the same, times from different caches

Choose a reason for hiding this comment

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

math.abs(t-time) is very close to 0. Am I getting this right?

Choose a reason for hiding this comment

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

you're right.

Choose a reason for hiding this comment

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

math.abs(t-time) is very close to 0

if time >= now - TIMEOUT, now - t < TIMEOUT when is it false?

jwrct

tomatoishealthy

long now = System.currentTimeMillis();
invToFetch.forEach((item, time) -> {
if (time < now - MSG_CACHE_DURATION_IN_BLOCKS * BLOCK_PRODUCED_INTERVAL) {
if (time < now - TIMEOUT) {

Choose a reason for hiding this comment

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

Is there an assumption: no more than 2w transactions will be broadcast within 15s?

Choose a reason for hiding this comment

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

At present, it is basically impossible to reach 20,000 within 15 seconds. We only focus on time and not TPS.