Change: Do not request one transaction at a time when checking for solidity

[karimodm]

Description

This change will make IRI request bigger portions of Tangle while exploring transactions for solidity.

Fixes #1310

Type of change

• Enhancement (a non-breaking change which adds functionality)

Checklist:

Please delete items that are not relevant.

• My code follows the style guidelines for this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[kwek20]

I noticed that after doing this, most milestones all turn solid at the same time (or in 5 seconds)
Although i'm not sure if that's bad, it seems that the 10 milestones list is kind of ineffective then and maybe we should change this to 2 or 3 to solidify the next milestone faster during syncing

[karimodm]

it seems that the 10 milestones list is kind of ineffective

Yeah, I've been experimenting with different size values of the milestone solidification pool; although the changes were not that significant as far as I could experience.

Anyhow this change refers to the Tangle traversing mechanism when checking solidity of either a milestone or a regular transaction. Instead of requesting a single branch as a result of a single checkSolidity call, this change would make more than one PREFILLED_SLOT end up in the requester queue, before the next call happens: I've experienced an improvement in solidification times.

[GalRogozinski]

After Discussing with @jakubcech we decided that this should be in 1.6.2 and not 1.6.1
We first want to add the stabilizing fixes and see that the system stabilizes and then add the improvements.

@karimodm if you can please add a unit test for the specific change it will be great!

[alon-e]

not sure if the following suggestion makes sense:
if it's a milestone, then continue, else break.

the reason being:
this mechanism can be used to fill the request queue with arbitrary data faster.

0
| \
0  ?
0
| \
0  ?
0
| \
0  ?
...

in the current code the first ? would be requested and stop (and if this ? does not exist the rest will never be requested, with this change all the historical ?s will be requested (up until the maxProccessedTransactions is reached).

I'm not sure this is such an issue, but my proposal would not change behavior for random transactions but achieve the improvement for syncing nodes.

[karimodm]

in the current code the first ? would be requested and stop (and if this ? does not exist the rest will never be requested.

Is this true? In the current code, upon the next call of this method, the first ? will already be in the requester queue, and the if (!transactionRequester.isTransactionRequested(hashPointer, milestone)) { will return false: so it will continue to request the next ?.

This being said I don't get your point on continue only if it's a milestone.

[GalRogozinski]

I think Alon's point is that all branches that are not confirmed by a milestone are "arbitrary data" and we don't want to have spend time or resources on them. The requester queue can fill up with sidetangle requests and thus hinder the solidification of "milestone branches".

However, if the TipsSolidifier is not enabled we are not supposed to be solidifying on "arbitrary data", and if it is enabled then maybe it is because the operator cares about "arbitrary data"?

Bottom line, due to the TipsSolidifier activation config I am with @karimodm.

[alon-e]

good point @karimodm, still this will take time to fill (1 tx per scan). I still think that this can be avoided by separating the cases; it will reduce the amount of change.

However, I don't insist on this, if @GalRogozinski thinks it's fine then I'm OK.

[karimodm]

still this will take time to fill (1 tx per scan)

Doesn't the continue here make the queue fill faster in any of the cases?

@GalRogozinski true on arbitrary data, this change combined with the requester freshness should fend off those scenarios though.

Alon approved