Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: traffic gen promise all #36

Merged
merged 3 commits into from
Jun 11, 2021

Conversation

nugaon
Copy link
Member

@nugaon nugaon commented May 21, 2021

As @vojtechsimetka correctly proposed, the cheque requests in traffic generation could happen concurently, that I made in this PR.

Nevertheless, the scripts won't exit from the infinite loop ever, because it overloads the Bee node and won't show up any incoming cheques on the destination node unless you break the infinite loop.

I guess it can be something related to how Bee handles its ongoing processes, but it seems like if the node processes and pushes continuously chunks, it cannot issue cheques.

@nugaon nugaon requested a review from a team May 21, 2021 12:22
@nugaon nugaon force-pushed the refactor/traffic-gen-promise-all branch from 1712170 to 40cd614 Compare May 21, 2021 12:28
Copy link
Contributor

@vojtechsimetka vojtechsimetka left a comment

Choose a reason for hiding this comment

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

Nevertheless, the scripts won't exit from the infinite loop ever, because it overloads the Bee node and won't show up any incoming cheques on the destination node unless you break the infinite loop.

Wow I wonder if there is some await missing so it just spams the bee nodes. If not, then I'd suggest to increase the sleep between uploads just to see how that will behave. I'm happy to jump on a pair programming session, so that we can report the findings to Bee devs.

scripts/gen-traffic.js Outdated Show resolved Hide resolved
Comment on lines +102 to 106
for(const [index, lastCashOut] of lastCashOuts.entries()) {
if(lastCashOut.uncashedAmount > 0) {
uncashedCheques.push(incomingCheque)
uncashedCheques.push(incomingCheques[index])
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Also suggesting one line which just stores the number of uncashed cheques (we don't really care about the amount, right? ).

const count = lastCashOuts.reduce((acc, {uncashedAmount}) => uncashedAmount > 0 ? acc +1 : acc, 0)

Copy link
Member Author

Choose a reason for hiding this comment

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

I would keep it as it is now for future request that requires some amount of debt from node, and it is quite simple now

@nugaon
Copy link
Member Author

nugaon commented May 21, 2021

Wow I wonder if there is some await missing so it just spams the bee nodes. If not, then I'd suggest to increase the sleep between uploads just to see how that will behave. I'm happy to jump on a pair programming session, so that we can report the findings to Bee devs.

with the prev setup in the base branch works, because with the gradual queries for cheques fills out this additional sleep time that it needs.

I created this branch for investigate this issue, but it won't be merged until the Bee client can handle the cheque issuances on load.

@nugaon
Copy link
Member Author

nugaon commented Jun 11, 2021

Long story short, because of the time-base settlement the cheques will issued a little later than before (~1-2 min) but actually they will be issued after a while, so everything seems fine regarding to this.
Also the behaviour of the cheque issuance was strange because the cheques immediately appeared after I stopped the data generation script, but it may will be detailed in the related bee issue, the important is it is not related to the bee couldn't send cheques during on high load.

@nugaon
Copy link
Member Author

nugaon commented Jun 11, 2021

I also raised the sleep time, because I noticed the cheques arrive earlier if the data upload is not continuous.

@nugaon nugaon merged commit 964fc63 into feat/parametric-traffic-gen Jun 11, 2021
nugaon added a commit that referenced this pull request Jun 14, 2021
* feat: parametric traffic generation

* refactor: host input splitting

Co-authored-by: Vojtech Simetka <vojtech@simetka.cz>

* docs: update readme

* refactor: typeo

* refactor: traffic gen promise all (#36)

* refactor: concurent promises

* refactor: simplify lastCashOutPromises

* refactor: raise timeout to get cheques sooner

Co-authored-by: Vojtech Simetka <vojtech@simetka.cz>
@AuHau AuHau deleted the refactor/traffic-gen-promise-all branch May 3, 2022 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants