Persist pending messages #2299

mfranciszkiewicz · 2018-03-06T13:11:52Z

extra database session.db for storing unsent messages and session details
tries to periodically restore sessions and send pending messages
identifies lost sessions by the ConnectionLost error from twisted
Session.send now returns a success boolean value
removes the send_unverified argument from Session.send in favor of the Session.can_be_unverified collection
adds node_info to TaskSession's Hello message

codecov · 2018-03-06T13:12:05Z

Codecov Report

Merging #2299 into develop will decrease coverage by 1.15%.
The diff coverage is 69.58%.

@@             Coverage Diff             @@
##           develop    #2299      +/-   ##
===========================================
- Coverage    89.81%   88.65%   -1.16%     
===========================================
  Files          177      178       +1     
  Lines        15556    15729     +173     
===========================================
- Hits         13971    13945      -26     
- Misses        1585     1784     +199

Default value changes to Session.dropped and Protocol.dropped

Remove session_exists

Minor refactoring

Exclude PendingObjectModel from collect_db_models

jiivan · 2018-03-20T14:36:25Z

related to: #2223

jiivan

The golem.network.pending part looks very complicated. Would it be possible to expose simple API/shortcuts as I suggested in https://github.com/golemfactory/golem/issues/2223#issuecomment-368491180 ? (pending.put(), pending.get(), pending.waiting())

jiivan · 2018-03-20T14:42:14Z

golem/network/pending.py

+class PendingMessage(PendingObjectModel):
+
+    type = IntegerField()
+    slots = MessageSlotsField()


This class should probably implement .as_message() method. (It should know how to create a message from itself)

jiivan · 2018-03-20T14:43:26Z

golem/network/pending.py

+    def get(cls,
+            key_id: str,
+            task_id: Optional[object] = ANY,
+            subtask_id: Optional[object] = ANY) -> Iterator[PendingMessage]:


To be consistent with .put() this should return Message iterator

jiivan · 2018-03-20T14:49:38Z

golem/network/pending.py

+        return reduce(operator.and_, clauses)
+
+
+class PendingTaskSession(PendingObjectModel):


Since in current implementation TaskSession instance is bound to a subtask, is it necessary to have separate PendingSession and PendingMessage? Logically we're interested in sending a pending message, and address:port should be accessible from PeerKeeper.

PendingMessage is enough to send the message, but might not be enough to correctly process the response and send the following messages; e.g. self.result_owner is required in some places.

mfranciszkiewicz · 2018-03-20T17:13:32Z

I don't think the API is that simple, since this is not a message queue. Sessions are lost on both ends, and sending a mid-protocol message requires consistency (on both ends). Pending task session's data may be redundant, but at least it's complete.

Please correct me if I'm missing something.

Elfoniok

LGTM, but why we need aux database?

Elfoniok · 2018-03-26T09:21:53Z

golem/network/p2p/peersession.py

@@ -116,15 +117,12 @@ def interpret(self, msg):
        )
        BasicSafeSession.interpret(self, msg)

-    def send(self, msg, send_unverified=False):
+    def send(self, msg) -> bool:
        """Send given message if connection was verified or send_unverified
           option is set to True.


Probably need to change docs

mfranciszkiewicz · 2018-03-26T10:28:06Z

@Elfoniok The main database is mildly congested at times and the second database is there in order not to make the situation worse.

mfranciszkiewicz · 2018-03-26T10:28:32Z

@jiivan Another approach would be to store session's state outside the TaskSession class and implement the message queue. Are there any plans for the latter?

mfranciszkiewicz added the in progress label Mar 6, 2018

mfranciszkiewicz changed the base branch from multiple_dbs to develop March 7, 2018 09:24

mfranciszkiewicz force-pushed the pending_messages branch from 8c24f2d to 71d25a9 Compare March 7, 2018 10:04

mfranciszkiewicz added 2 commits March 7, 2018 17:03

Pending sessions and messages storage

fa73240

Persist pending messages

58975ea

mfranciszkiewicz force-pushed the pending_messages branch from 414de98 to d2345cd Compare March 7, 2018 17:48

mfranciszkiewicz added 7 commits March 7, 2018 19:41

Store and restore task session state

eab549c

Default value changes to Session.dropped and Protocol.dropped

Order pending messages by created_date

cdf930b

Remove session_exists

Send node_info in TaskSession's Hello message

46bfb66

Minor refactoring

Add "excluded" argument to collect_db_models

ebe3ac8

Exclude PendingObjectModel from collect_db_models

Refactoring

3e33d24

Sync pending sessions

1cb29f1

Linter

f20182f

mfranciszkiewicz force-pushed the pending_messages branch 2 times, most recently from d288c6e to febc8ea Compare March 7, 2018 19:13

Conditional pending_messages

b776f4d

mfranciszkiewicz force-pushed the pending_messages branch 2 times, most recently from 0ee1235 to d28862f Compare March 7, 2018 23:05

mfranciszkiewicz added 2 commits March 8, 2018 01:38

Cleanup PendingSessionMessages

351488e

Fixed version of pycryptodome (3.4.11)

e2fec91

mfranciszkiewicz force-pushed the pending_messages branch from d28862f to e2fec91 Compare March 8, 2018 00:38

mfranciszkiewicz and others added 5 commits March 9, 2018 13:40

Merge remote-tracking branch 'origin/develop' into pending_messages

b70b76b

Rename node_id to key_id

b13f23e

Convert node_info to Node in TaskServer.sync_sessions

350d261

Merge remote-tracking branch 'origin/develop' into pending_messages

22d77b7

Linter

12726b0

mfranciszkiewicz added needs review and removed in progress labels Mar 12, 2018

Bump SESSION_CHECK_INTERVAL to 20s

0e683d8

Merge remote-tracking branch 'origin/develop' into pending_messages

a9347b9

badb added in progress and removed needs review labels Mar 15, 2018

mfranciszkiewicz added 2 commits March 17, 2018 20:24

Merge remote-tracking branch 'origin/develop' into pending_messages

386ace1

Rename log to logger in client.py

7764853

mfranciszkiewicz requested review from jiivan and Elfoniok March 17, 2018 19:31

mfranciszkiewicz added needs review and removed in progress labels Mar 17, 2018

jiivan reviewed Mar 20, 2018

View reviewed changes

Review, part 1

9523035

Elfoniok approved these changes Mar 26, 2018

View reviewed changes

mfranciszkiewicz closed this Aug 23, 2018

ghost removed the needs review label Aug 23, 2018

mfranciszkiewicz deleted the pending_messages branch May 13, 2019 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist pending messages #2299

Persist pending messages #2299

mfranciszkiewicz commented Mar 6, 2018 •

edited

Loading

codecov bot commented Mar 6, 2018 •

edited

Loading

jiivan commented Mar 20, 2018

jiivan left a comment

jiivan Mar 20, 2018

mfranciszkiewicz Mar 20, 2018

jiivan Mar 20, 2018

mfranciszkiewicz Mar 20, 2018

jiivan Mar 20, 2018

mfranciszkiewicz Mar 20, 2018 •

edited

Loading

mfranciszkiewicz commented Mar 20, 2018

Elfoniok left a comment

Elfoniok Mar 26, 2018

mfranciszkiewicz commented Mar 26, 2018

mfranciszkiewicz commented Mar 26, 2018

		return reduce(operator.and_, clauses)


		class PendingTaskSession(PendingObjectModel):

Persist pending messages #2299

Persist pending messages #2299

Conversation

mfranciszkiewicz commented Mar 6, 2018 • edited Loading

codecov bot commented Mar 6, 2018 • edited Loading

Codecov Report

jiivan commented Mar 20, 2018

jiivan left a comment

Choose a reason for hiding this comment

jiivan Mar 20, 2018

Choose a reason for hiding this comment

mfranciszkiewicz Mar 20, 2018

Choose a reason for hiding this comment

jiivan Mar 20, 2018

Choose a reason for hiding this comment

mfranciszkiewicz Mar 20, 2018

Choose a reason for hiding this comment

jiivan Mar 20, 2018

Choose a reason for hiding this comment

mfranciszkiewicz Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

mfranciszkiewicz commented Mar 20, 2018

Elfoniok left a comment

Choose a reason for hiding this comment

Elfoniok Mar 26, 2018

Choose a reason for hiding this comment

mfranciszkiewicz commented Mar 26, 2018

mfranciszkiewicz commented Mar 26, 2018

mfranciszkiewicz commented Mar 6, 2018 •

edited

Loading

codecov bot commented Mar 6, 2018 •

edited

Loading

mfranciszkiewicz Mar 20, 2018 •

edited

Loading