Change in vdsm[master]: stomp: reroute messages to different process
From Yaniv Bronhaim : Yaniv Bronhaim has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65846/5//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-10-28 12:48:53 +0200 Line 4: Commit: Piotr Kliczewski Line 5: CommitDate: 2016-11-17 11:13:59 +0100 Line 6: Line 7: stomp: reroute messages to different process I'd change the title to "Introducing redirect in stomp protocol header" Line 8: Line 9: When a client subscribes it can define which messages should be rerouted Line 10: to it based on a custom stomp header. We use the header "redirect" Line 11: which holds a list of a verbs. Every time a call to a verb arrives we -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Roman Mohr Gerrit-Reviewer: Stuart Gott Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
From Yaniv Bronhaim : Yaniv Bronhaim has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65846/5//COMMIT_MSG Commit Message: Line 24: to that process we need to send SUBSCRIBE frame with header Line 25: Line 26: 'redirect': 'VM.create' Line 27: Line 28: This would cause vdsm to forward vm creation requests to this process. who needs that? do we have a scenario for that? bugzilla? I'm trying to think who would want to subscribe for such thing and can't find anything.. sorry, maybe I don't have enough info about this scope Line 29: Line 30: Line 31: Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Roman Mohr Gerrit-Reviewer: Stuart Gott Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
From Yaniv Bronhaim : Yaniv Bronhaim has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/65846/4//COMMIT_MSG Commit Message: Line 14: vdsm would not process it. Line 15: Line 16: Once the message is processed by the client it can send a response by Line 17: using standard engine response queue and vdsm would send it to the Line 18: engine. can you give an example for usage? what client needs such thing? Line 19: Line 20: This mechanism allows to transparently forward messages to clients. Line 21: Line 22: https://gerrit.ovirt.org/#/c/65846/4/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py: Line 245: def _handle_internal(self, dispatcher, frame): Line 246: """ Line 247: We need to build response dictionary which maps message id Line 248: with destination. For legacy mode we use known 3.5 destination Line 249: or for standard mode we use 'reply-to' header. caught another 3.5 legacy that we can remove? Line 250: """ Line 251: requests = [] Line 252: try: Line 253: req_dest = frame.headers.get(stomp.Headers.REPLY_TO) -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Roman Mohr Gerrit-Reviewer: Stuart Gott Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
From Yaniv Bronhaim : Yaniv Bronhaim has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/65846/3//COMMIT_MSG Commit Message: Line 7: stomp: reroute messages to different process Line 8: Line 9: When a client subscribes it can define which messages should be rerouted Line 10: to it based on a custom stomp header. Whenever vdsm receives message for Line 11: a verb specified it sends it to that client instead of processing it. I'll copy my general comment here so you won't miss it - ""This is quite an important extension to vdsm's stomp protocol logic. I don't know if the "redirect" header is part of stomp rfc (if it is please refer to it and add a link to the description). In any case, please add detailed explanation about the implementation - in each subscription you hold a list of redirects, how you use this list and when. it will be easier to understand your design here"" Line 12: Line 13: Line 14: Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d https://gerrit.ovirt.org/#/c/65846/3/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py: Line 255: except Exception: Line 256: # let json server process issue Line 257: pass Line 258: for request in requests: Line 259: dispatcher.connection.handleMessage(request) > Please suggest how to do it. We have bulk processing of messages on vdsm si separate patch Line 260: Line 261: def _handle_destination(self, dispatcher, req_dest, frame, request): Line 262: """ Line 263: We could receive single message or batch of messages. We need https://gerrit.ovirt.org/#/c/65846/3/tests/stompAdapterTests.py File tests/stompAdapterTests.py: Line 305: adapter.handle_frame(TestDispatcher(adapter), frame) Line 306: Line 307: data = adapter.pop_message() Line 308: self.assertIsNot(data, None) Line 309: request = JsonRpcRequest.fromRawObject(data) > It is. We need to to have the tests pass. how does the change brake this test ? Line 310: self.assertEquals(request.method, 'Host.getAllVmStats') Line 311: self.assertTrue(len(ids) == 1) Line 312: Line 313: def test_send_no_destination(self): -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Roman Mohr Gerrit-Reviewer: Stuart Gott Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
From Yaniv Bronhaim : Yaniv Bronhaim has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 3: Code-Review-1 (2 comments) This is quite important extension to vdsm's stomp protocol logic. I don't know if the "redirect" header is part of stomp rfc (if it is please refer to it and add a link to the description). In any case, please add detailed explanation about the implementation - in each subscription you hold a list of redirects, how you use this list and when. it will be easier to understand your design here https://gerrit.ovirt.org/#/c/65846/3/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py: Line 255: except Exception: Line 256: # let json server process issue Line 257: pass Line 258: for request in requests: Line 259: dispatcher.connection.handleMessage(request) how handling more than one request is related to redirects? maybe it is worth separate patch? Line 260: Line 261: def _handle_destination(self, dispatcher, req_dest, frame, request): Line 262: """ Line 263: We could receive single message or batch of messages. We need https://gerrit.ovirt.org/#/c/65846/3/tests/stompAdapterTests.py File tests/stompAdapterTests.py: Line 305: adapter.handle_frame(TestDispatcher(adapter), frame) Line 306: Line 307: data = adapter.pop_message() Line 308: self.assertIsNot(data, None) Line 309: request = JsonRpcRequest.fromRawObject(data) is it related to the patch? Line 310: self.assertEquals(request.method, 'Host.getAllVmStats') Line 311: self.assertTrue(len(ids) == 1) Line 312: Line 313: def test_send_no_destination(self): -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Roman Mohr Gerrit-Reviewer: Stuart Gott Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
gerrit-hooks has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 3: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, no bug url/s found * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
gerrit-hooks has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 2: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, no bug url/s found * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
Piotr Kliczewski has uploaded a new change for review.
Change subject: stomp: reroute messages to different process
..
stomp: reroute messages to different process
When a client subscribes it can define which messages should be rerouted
to it based on a custom stomp header. Whenever vdsm receives message for
a verb specified it sends it to that client instead of processing it.
Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d
Signed-off-by: Piotr Kliczewski
---
M lib/yajsonrpc/stomp.py
M lib/yajsonrpc/stompreactor.py
M tests/stompAdapterTests.py
3 files changed, 161 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/65846/1
diff --git a/lib/yajsonrpc/stomp.py b/lib/yajsonrpc/stomp.py
index 558c519..99d09ac 100644
--- a/lib/yajsonrpc/stomp.py
+++ b/lib/yajsonrpc/stomp.py
@@ -563,6 +563,7 @@
self._valid = True
self._message_handler = message_handler
self._destination = destination
+self._redirects = []
def handle_message(self, frame):
self._message_handler(self, frame)
@@ -576,6 +577,13 @@
def set_message_handler(self, handler):
self._message_handler = handler
+def set_redirects(self, redirects):
+self._redirects = redirects
+
+@property
+def redirects(self):
+return self._redirects
+
@property
def id(self):
return self._subid
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py
index 42d7d01..61bb98c 100644
--- a/lib/yajsonrpc/stompreactor.py
+++ b/lib/yajsonrpc/stompreactor.py
@@ -15,7 +15,7 @@
from __future__ import absolute_import
import logging
-from collections import deque
+from collections import deque, defaultdict
from uuid import uuid4
import functools
@@ -68,6 +68,7 @@
self._outbox = deque()
self._sub_dests = sub_map
self._req_dest = req_dest
+self._redirects = defaultdict(list)
self._sub_ids = {}
request_queues = config.get('addresses', 'request_queues')
self.request_queues = request_queues.split(",")
@@ -128,6 +129,7 @@
self.log.info("Subscribe command received")
destination = frame.headers.get("destination", None)
sub_id = frame.headers.get("id", None)
+redirect_value = frame.headers.get("redirect", None)
if not destination or not sub_id:
self._send_error("Missing destination or subscription id header",
@@ -145,6 +147,12 @@
self._sub_dests[destination].append(subscription)
self._sub_ids[sub_id] = subscription
+
+if redirect_value:
+redirects = redirect_value.split(",")
+subscription.set_redirects(redirects)
+for redirect in redirects:
+self._redirects[redirect].append(subscription)
def _send_error(self, msg, connection):
res = stomp.Frame(
@@ -188,17 +196,26 @@
subs = self._sub_dests[subscription.destination]
if len(subs) == 1:
del self._sub_dests[subscription.destination]
+self._remove_redirects(subscription)
else:
if subscription in subs:
subs.remove(subscription)
+self._remove_redirects(subscription)
+
+def _remove_redirects(self, subscription):
+if subscription.redirects:
+for redirect in subscription.redirects:
+reds = self._redirects[redirect]
+if len(reds) == 1:
+del self._redirects[redirect]
+else:
+reds.remove(subscription)
def _cmd_send(self, dispatcher, frame):
destination = frame.headers.get(stomp.Headers.DESTINATION, None)
if destination in self.request_queues:
# default subscription
-self._handle_internal(dispatcher,
- frame.headers.get(stomp.Headers.REPLY_TO),
- frame.body)
+self._handle_internal(dispatcher, frame)
return
else:
try:
@@ -224,31 +241,42 @@
)
subscription.client.send_raw(res)
-def _handle_internal(self, dispatcher, req_dest, request):
+def _handle_internal(self, dispatcher, frame):
"""
We need to build response dictionary which maps message id
with destination. For legacy mode we use known 3.5 destination
or for standard mode we use 'reply-to' header.
"""
+requests = []
try:
-self._handle_destination(dispatcher, req_dest, json.loads(request))
+req_dest = frame.headers.get(stomp.Headers.REPLY_TO)
+requests = self._handle_destination(dispatcher, req_dest,
+frame, json.loads(frame.body))
except Exception:
# let json server
Change in vdsm[master]: stomp: reroute messages to different process
Piotr Kliczewski has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 1: Verified that changes do not break functionality and by running UT. Now need external client to make sure it works as it should. -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: stomp: reroute messages to different process
gerrit-hooks has posted comments on this change. Change subject: stomp: reroute messages to different process .. Patch Set 1: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, no bug url/s found * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I622cb7f3b39a19314b7de4c325a62fa47faeaa4d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
