Experiment 1.1

[drew2a]

Popularity Community Experiment

Issue: #5580
Raw result data from the last build: result.csv
Build-to-build visualisation: plot in Jenkins
Jenkins job is scheduled to run every day at 8:00.

initial_filling.py

initial_filling.py

Description

This script calculate velocity of torrents list filling.

Given: the real network.

Action:

• add a new node
• every N seconds check how long torrent's list are

Result will be stored in a csv file.
Example:

time_in_sec,total,alive 
10,313,27 
20,313,27 
30,320,27 

Here alive means have at least 1 seeder.

Experiment results

Velocity for the first 10 minutes
Total: 572
Alive: 40

Velocity for the first 30 minutes
Total: 2817
Alive: 145

Only alive:

Usage

python3 initial_filling.py [-i <check_interval_in_sec>] [-t <timeout_in_sec>] [-f <output_file.csv>]

Where:

• check_interval_in_sec means how frequently we check the torrent list
• timeout_in_sec means a time that the experiment will last
• output_file.csv means a path and a result file name

Example

python3 initial_filling.py -i 60
python3 initial_filling.py -i 60 -t 900
python3 initial_filling.py -i 60 -t 900 -f result.csv

[ghost]

Congratulations 🎉. DeepCode analyzed your code in 0.869 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

[devos50]

retest this please

[devos50]

@drew2a looks like there was a zombie Tribler process running on the Windows 64-bit testing machine. I killed it and the GUI tests seems to pass now 👍

[drew2a]

@devos50 thanks :)

[devos50]

Nice progress! It appears that many of the gossiped information in the PopularityCommunity indicates dead torrents. Note, however, that the current approach of checking the health of a single torrent is somewhat unreliable. Specifically, many torrents in Tribler do not have a UDP/TCP tracker specified. We check the health of such tracker-less torrents by naively joining the swarm and by verifying that the DHT is introducing some BitTorrent peers to us. If so, we mark the torrent as healthy. This behaviour is implemented here.

A promising approach is to utilise the BEP33 protocol that has libtorrent DHT peers maintain two bloomfilters (one for the leeches and one for the seeders) to estimate the size/health of a swarm. Unfortunately, this protocol is not natively supported by libtorrent and adding support requires some modifications to the upstream code. I implemented the protocol in native libtorrent a while ago, which is further discussed here. I also performed a few DAS5 experiments and some graphs can be found here. Using BEP33 resulted in less dead torrents, at the cost of additional bandwidth requirements.

Maybe you can consider this when designing follow-up experiments :)

[devos50]

Also, I'm not entirely sure if we want to add this code to our main repository. Scripts that are exclusively used during experiments usually are usually included in our Gumby experiment framework.

[qstokkink]

Also, I'm not entirely sure if we want to add this code to our main repository. Scripts that are exclusively used during experiments usually are usually included in our Gumby experiment framework.

Yes, this definitely should go into the Gumby repo. You can then also get rid of ~90% of the boilerplate code in this PR then.

[ichorid]

Yes, this definitely should go into the Gumby repo. You can then also get rid of ~90% of the boilerplate code in this PR then.

But then we would not have seen this PR, which is easily the most beautiful, disciplined, clean and formally written thing to ever appear on Tribler "Pull Requests" page 👍 👼

This tells something about our average code quality and priorities, BTW 🤣

[qstokkink]

But then we would not have seen this PR, which is easily the most beautiful, disciplined, clean and formally written thing to ever appear on Tribler "Pull Requests" page 👍 👼

We can make standardized PR message templates, I've done this for IPv8 as well. The issue is that developers are usually too lazy to fill out an entire questionnaire about some trivial PR. I'd say you need a well-thought-out selection of different PR templates that fit the different types of contributions if you want to maintain any form of standard.

This tells something about our average code quality and priorities, BTW 🤣

How does a PR message reflect code quality? I can see how it would say something about our contribution pipeline's quality assurance though. Writing a proper PR message shows respect to your reviewer's time and I appreciate that as well.

[ichorid]

I mean, look at the code quality. That's what I was talking about, not just PR message.

[qstokkink]

@ichorid I might be misunderstanding your use of the "🤣" emoji then. I'd say the code quality is pretty good if you can launch a clean-slate Tribler instance like this with custom Community logic hooked in.

[ichorid]

@qstokkink , my ":rofl:" emojis meant literally the following: " @drew2a 's code quality and presentation is much higher than what we are typically used to in Tribler, and that becomes especially funny if you consider that this is just some throw-away experimental PR"

[qstokkink]

@qstokkink , my "🤣" emojis meant literally the following: " @drew2a 's code quality and presentation is much higher than what we are typically used to in Tribler, and that becomes especially funny if you consider that this is just some throw-away experimental PR"

Thanks for the clarification, that is quite the insult. But, I'm not seeing it. Could you give some examples in this PR of what you consider better than existing code? In order to not make any 3rd party casualties, I'll put my own ass on the line: you can use my code of https://github.com/Tribler/tribler/pull/5581/files for reference.

Regarding the code of this PR, I see shadowing of class-space attribute names being shadowed by instance fields (e.g. self._configuration on line 51 shadowing ObservablePopularityCommunity._configuration on line 48). What is the team's stance on this? Personally, I don't like this as it can lead to stuff like this:

class A:

    some_field = None
    
    def __init__(self, some_field):
        self.some_field = some_field
        

class B:

    some_field = None
    
    def __init__(self, some_field):
        self.some_fie1d = some_field
        

a = A(1)
b = B(1)

print(a.some_field)  #  prints 1
print(b.some_field)  #  prints None

a.some_field = 2
b.some_field = 2

print(a.some_field)  #  prints 2
print(b.some_field)  #  prints 2

[ichorid]

Thanks for the clarification, that is quite the insult.

@qstokkink , now I don't see an insult... Could you please clarify?

Now to the code quality. I would definitely consider the following features a complete overkill for a one-shot experiment:

• the separate Configuration class
• the def __str__(self) magic method in that class
• the proper breakdown of the ObservablePopularityCommunity's __init__ into small, readable functions that are only called once(!). Typically, we tend to put experimental code into a single long function (because we are more used to single-shot experiments than to code reuse).
• the proper _parse_argv function (for an experiment that is probably going to be run just a couple of times and by hand)
• the readme.md file with a command-line format documentation
• the consistent usage of _underscore notation

Nonetheless @drew2a added the stuff. Which definitely tells us about how pedantic the author is, be it for good or bad...

[ichorid]

Regarding the class variable shadowing, I think it is unacceptable, except for extreme cases like compatiblity stuff, testing, etc. Class variables are just that - class variables.

@qstokkink , your example cleverly exploits 1l amibiguity. However, we have IDE warnings and unit tests to keep that at bay.

This only shows that, while being a very disciplined programmer, @drew2a is not too familiar with Python development quirks... That's why @qstokkink 's input is especially valuable in reviewing @drew2a 's code 😉

[xoriole]

I quite like @drew2a coding style. I approve the PR and I'll adopt some of the styles in my future PR.

[drew2a]

@ichorid @qstokkink gens, thank you for your comments.

[devos50]

One proposal: let's add this directory to the root directory in the repo.

Also, don't forget to rebase this PR 👍

[egbertbouman]

This tells something about our average code quality and priorities, BTW 🤣

That's just really unnecessary. You know, it's actually possible to compliment a colleague's coding style, without insulting the rest of the team.

[drew2a]

@qstokkink thank you for your proposals. I've implement them for avoiding potential "shadowing issues".

I'm still not "pythonic" enough, so feel free to correct me.

@devos50 code has been moved to the root.

[ichorid]

@qstokkink , @egbertbouman , OK, dear colleagues. I officially apologize for insulting your coding style. I did not meant anyone personally. Probably, my judgement was based on too much student code refactoring over the last couple of years...

[qstokkink]

@drew2a looking good 👍 If at all possible, I will try to prevent you from making the mistakes I have in the past (hopefully with some fun code snippets and not links to API docs 😛).

@ichorid no worries, I will assume that was unhappy wording of good intentions. Thank you for specifying exactly what you meant with quality coding. Though it is an important end result, we cannot improve our project on sentiment and I had to press you on what components you were unhappy with exactly.

[kozlovsky]

Looks good to me

[devos50]

retest this please

[devos50]

retest this please

[devos50]

retest this please

[devos50]

retest this please

[devos50]

retest this please

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 1 Security Hotspot to review)
0 Code Smells

No Coverage information
0.0% Duplication