-
Notifications
You must be signed in to change notification settings - Fork 443
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
Singleton and Member cache cleanup #79
Singleton and Member cache cleanup #79
Conversation
All Singleton methods are no longer available: - dispersy.has_instance - dispersy.get_instance - dispersy.del_instance The Dispersy instance is available directly from the LaunchManyCore, or though the session instance (session.lm.dispersy). Also, DummyMember and Member instances should only be created through Dispersy using: - dispersy.get_temorary_member_from_id (returns a DummyMember) - dispersy.get_new_member - dispersy.get_member - dispersy.get_members_from_id - dispersy.get_member_from_database_id
…e ChannelCommunity construtor.
- dispersy_database -> dispersy.database - join_community(master, ...) -> join_community(dispersy, master, ...) - DummyMember(cid) -> dispersy.get_temporary_member_from_id(cid)
The origional change should not have been committed in the first place.
There is still some work that needs to be discussed. Namely, where the callback should start/stop. Up until now the callback was created and started by the LaunchManyCore. Hence, the callback should also be stopped by the LaunchManyCore. If we want, we can move both the start and stop to Dispersy. The same thing goes for the Endpoint, this is currently created in the LaunchManyCore and passed to Dispersy as a parameter. Whatever we decide, we need to take into account that Dispersy is also started from dispersy/tool/main.py (and possibly other locations). When running from main.py dispersy should not call callback.start/stop since this would create a new thread, but instead run callback.loop on the main thread. Finally, StandaloneEndpoint has a start/stop that is called from main.py. |
I'm trying to reply to the comment of Boudewijn. |
After a chat with Niels we agree on the following:
Hence, we need to add start and stop methods to all Endpoint classes, even though only StandaloneEnpoint actualy uses it, and we will need to add a NoThreadCallback class that doesn't start a thread to use within dispersy/tool/main.py. |
I am not seeing any channels so something is broken, unfortunately I am not getting any errors. I had the same thing a few days ago, and Egbert could see errors... |
It is not possible to load communities during Dispersy.start because the community parameters depend on having a Member, and creating a Member depends on Dispery.start having finished. Hence Dispersy.start no longer loads all communities, instead, this can now be done by define_auto_load.
…persystop Conflicts: Tribler/dispersy
I have seen each test in https://jenkins.tribler.org/jenkins/job/Test_tribler_full-ui-run_stop working on my own PC at least once. test_libtorrent_download.py:test_stopresumedelete seems the most error prone, perhaps because it relies on external resources such as torrent files and peers. However, this test inconsistency is unrelated to this pull request. I see no more issues holding this merge back. Please comment if you feel differently. |
The only strange thing I see is that the dht cannot bind on the socket. 2013/4/15 Boudewijn [email protected]
|
As far as I know the Dispersy port number hasn't changed for a long time. But I recall that a relatively recent change to the DHT port number used the default port - 1... and DEFAULTPORT=7760. I'm trying to find the change to confirm... |
Confirmed. It wasn't a problem before because Dispersy would add 1 to the port and try again, but since we now call Dispersy start earlier it is becoming the dht's problem. And the dht can't handle it. Ive change it to using port 7757 (7758 is already taken by swift) |
Great, maybe run the ui tests again to check if the problem is resolved. 2013/4/15 Boudewijn [email protected]
|
No, didn't resolve the existing issue. I suggest we attempt to solve below bug in its own issue, since this is unrelated to this pull request.
|
The delInstance of the superclass was called. And since delInstance removed cls.__single, this would result in a completely different member since __single is renamed into __CLASSNAME_single.
I've fixed a bug in the cache db, see boudewijn-tribler@0a3b2d4. The database singleton was never removed/closed at tribler shutdown, and the testcases kept using the same database cursor over and over. This solves an exception that occurs when the final call to Anyway. The test |
Gui test seems to be working now, I'll ask Egbert to look into the 2013/4/16 Boudewijn [email protected]
|
Unless anyone feels the duplicate download is related to this pull request, I suggest we discuss improvement/cleanup to sqlitecachedb.py on a different issue. Also, performing this merge will allow us to continue with the dprint modifications without causes tons of merge problems. |
It's not, looking at this ( 2013/4/16 Boudewijn [email protected]
|
@@ -1,6 +1,6 @@ | |||
[submodule "Tribler/dispersy"] | |||
path = Tribler/dispersy | |||
url = https://github.com/Tribler/dispersy.git | |||
url = https://github.com/boudewijn-tribler/dispersy.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be here, we are going to do both Dispersy and Tribler merge at the same time, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, you are right. but when I remove it the Jenkins task will point to the wrong repro. So it should be changed just before the merge.
Please enlighten me if there was another (better) way to do this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of any, right now :)
Singleton and Member cache cleanup
All Singleton methods are no longer available:
The Dispersy instance is available directly from the LaunchManyCore,
or though the session instance (session.lm.dispersy).
Also, DummyMember and Member instances should only be created through
Dispersy using: