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

Avoid using fixed-seed prng #29

Merged
merged 1 commit into from
Feb 17, 2022
Merged

Avoid using fixed-seed prng #29

merged 1 commit into from
Feb 17, 2022

Conversation

sporksmith
Copy link
Contributor

libc's rand is seeded with 1 unless otherwise seeded with srand.
In particular this was causing all tgen clients with the same peer list
to choose peers in the same order.

libc's `rand` is seeded with `1` unless otherwise seeded with `srand`.
In particular this was causing all tgen clients with the same peer list
to choose peers in the same order.
@sporksmith
Copy link
Contributor Author

This was definitely causing perf clients to all hit the same servers in lockstep:

grep -h -E 'Created new stream' perfclient*exit/*.tgen.*.stdout | sort
perfclient-new-streams.txt

I'm not sure yet whether it affects markov clients. Strangely in the sim I'm looking at I only see the "Created new stream" string in markovclient1exit...

@sporksmith
Copy link
Contributor Author

sporksmith commented Feb 17, 2022

Strangely in the sim I'm looking at I only see the "Created new stream" string in markovclient1exit...

Ah, it looks like that's because only markovclient1's tgen logs at info level.

--- markovclient1exit/tgenrc.graphml	2022-02-17 13:51:47.345649231 -0600
+++ markovclient2exit/tgenrc.graphml	2022-02-17 13:51:47.329649146 -0600
@@ -12,20 +12,20 @@
 <key id="d1" for="node" attr.name="time" attr.type="string"/>
 <key id="d0" for="node" attr.name="loglevel" attr.type="string"/>
 <graph edgedefault="directed"><node id="start">
-  <data key="d0">info</data>
-  <data key="d1">19</data>
+  <data key="d0">message</data>
+  <data key="d1">18</data>
   <data key="d2">localhost:9050</data>

@sporksmith
Copy link
Contributor Author

FWIW this doesn't seem to ultimately make much difference, at least in a scale=0.001 network. There are some other things going on here (like locally adding more perf nodes), but the only difference between these 2 sims is using this patch or not in tgen.
tornet.plot.pages.pdf

Copy link
Member

@robgjansen robgjansen left a comment

Choose a reason for hiding this comment

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

Woo! Good find :)

For posterity:

I don’t think it affects the Markov clients because in that case we are seeding the Markov process using values that are set in the config file that are different for every client.

The Markov model seed is called markovmodelseed in the tgen config file. The explanation from the docs :

The seed that will be used to initialize a pseudorandom number generator (prng) that will generate seeds for all Markov models created for this action. If unspecified, tgen initializes the prng using a seed generated by a global prng that was randomly seeded.

So as long as we're setting that seed from the config file, and are using a different seed for each client, we'll generate different delays on their streams so they won't exactly overlap.

@sporksmith
Copy link
Contributor Author

Discussed some more out of band - it's not clear that markovmodelseed ends up getting used for peer selection, but yeah if nothing else the randomized timings, packet sizes, etc. should mitigate it.

@sporksmith sporksmith merged commit 3c432a6 into shadow:main Feb 17, 2022
@sporksmith sporksmith deleted the random branch February 17, 2022 23:58
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.

2 participants