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

feat: parametric traffic gen #35

Merged
merged 5 commits into from
Jun 14, 2021
Merged

feat: parametric traffic gen #35

merged 5 commits into from
Jun 14, 2021

Conversation

nugaon
Copy link
Member

@nugaon nugaon commented May 21, 2021

Traffic generation script had to be improved by passing the cheque numbers that is desired on a particular Bee node.

For that the traffic generation call structure changed to the following:

npm run gen:traffic -- [MIN_CHEQUE_NUMBER] [BEE_URL;BEE_DEBUG_URL] [BEE_URL;BEE_DEBUG_URL] ...

By that the script can exit from the infinite loop after the count of the incoming cheques on BEE_DEBUG_URL reaches the given MIN_CHEQUE_NUMBER. The random data will be generated on BEE_URL as it happened so far.

it is the continuation of the #28 PR

@nugaon nugaon requested a review from a team May 21, 2021 08:23
@nugaon nugaon force-pushed the feat/parametric-traffic-gen branch from bb0e4f8 to eea6ea6 Compare May 21, 2021 08:24
Comment on lines 98 to 106
for(const incomingCheque of incomingCheques) {
const lastCashOut = await beeDebug.getLastCashoutAction(incomingCheque.peer)
if(lastCashOut.uncashedAmount > 0) {
uncashedCheques.push(incomingCheque)
}
}

beesUncashedCheques.push(uncashedCheques)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This can potentially take long as you process one async call at time (not probably a problem locally but if you want to use it on real network). My suggestion would be to rewrite this into a map that returns array of promises. Then you can just await on all of them with await Promise.all.

Then you could just reduce this to the amount of cheques per peer.

Comment on lines +125 to +127
if(hosts.length === 0) {
hosts = [ 'http://localhost:1633;http://localhost:11635' ]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

My personal preference would be to already parse it here and then pass it as array of objects. But I do acknowledge it's not necessary:

[
    { beeApi: ... , beeDebugApi: ... },
    { beeApi: ... , beeDebugApi: ... },
]

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, actually I planned to resolve it in genTrafficLoop function originally. I think it is in a great place there as it is now.

scripts/gen-traffic.js Outdated Show resolved Hide resolved
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.

I think it's fine as is, it gets the job done. One thing we should be mindful about (probably in next iteration) is that the postage stamps may be all used up so no new uploads are made.

Co-authored-by: Vojtech Simetka <vojtech@simetka.cz>
scripts/gen-traffic.js Outdated Show resolved Hide resolved
nugaon and others added 3 commits May 21, 2021 14:26
* refactor: concurent promises

* refactor: simplify lastCashOutPromises

* refactor: raise timeout to get cheques sooner
@nugaon nugaon merged commit e163745 into master Jun 14, 2021
@nugaon nugaon deleted the feat/parametric-traffic-gen branch June 20, 2021 11:04
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

3 participants