search

flipkart-incubator / phantom

Phantom is a high performance proxy for accessing distributed services. It is an RPC system with support for different transports and protocols. Phantom is inspired by Twitter Finagle clients and builds on the capabilities of technologies like Netty, Unix Domain Sockets, Netflix Hystrix and Spring. Phantom proxies have been used to serve several hundred million API calls in production deployments at Flipkart.
50 stars 27 forks source link

Implemented ServiceProxy Event Producer where in UDSOIO,Thrift and HTTP ... #1

Closed amanhigh closed 11 years ago

amanhigh commented 11 years ago

...Handlers would publish events which interested Consumers can subscribe to.

regunathb commented 11 years ago

Aman, Please refactor as discussed. Also, the independent module for the ProxyEventProducer appears to be an overkill.

amanhigh commented 11 years ago

Right now Enum helps Splitting the event based on its origin namely THRIFT,HTTP and TASK Origins. This later helps to separate events being published to various end points. Removing ENUM we would loose this separation. You have anything else in mind ?

amanhigh commented 11 years ago

That Cast would no longer be there after the refactor as publisher would be changed. ServiceProxyEvent i just made fields protected inline with its parent PlatformEvent. Can go for private there.

shashwata commented 11 years ago

Right now Enum helps Splitting the event based on its origin namely THRIFT,HTTP and TASK Origins. This later helps to separate events being published to various end points. Removing ENUM we would loose this separation. You have anything else in mind ?

Your event already has a eventSource string. Can't it be used? Enums are more restrictive here. If users are adding more task handlers independent of the platform provided task handlers, then this enum is useless for them. For instance UDSOIO events are not recognized here.

amanhigh commented 11 years ago

Event Source and Enum are for different purposes. Event Source is exact class name which was executing the generated event. For Eg it can be Sherlock Task Handler, Service Proxy Task Handler etc for all these event sources Event Type is common i.e. TASK_HANDLER. As you said we could use Event Source to split however then a consumer interested in subscribing to all events of TASK_HANDLER would have to define too many endpoints when subscribing. Every time a new Service Handler is introduced consumer has to update list of subscribed endpoints.

For the time being we have just made three end points namely events of TASK,HTTP & Thrift. Going forward if use case arise we could split based on Source and git rid of Event Type. Also we have plans to split into success and failure events. eg TASK/SUCCESS, TASK/FAILURE etc etc.

shashwata commented 11 years ago

What if I need to only listen to Sherlock Task handler? What you need is a topic registration and not event type registration. Publishers publish to a topic and subscribers listen to a topic. It seems the topics created right now (ie TASK, HTTP and Thrift) are more of a convenience for Flipkart rather than being a generic solution.

On Wed, Oct 30, 2013 at 11:49 AM, Amanpreet Singh notifications@github.comwrote:

Event Source and Enum are for different purposes. Event Source is exact class name which was executing the generated event. For Eg it can be Sherlock Task Handler, Service Proxy Task Handler etc for all these event sources Event Type is common i.e. TASK_HANDLER. As you said we could use Event Source to split however then a consumer interested in subscribing to all events of TASK_HANDLER would have to define too many endpoints when subscribing. Every time a new Service Handler is introduced consumer has to update list of subscribed endpoints.

For the time being we have just made three end points namely events of TASK,HTTP & Thrift. Going forward if use case arise we could split based on Source and git rid of Event Type. Also we have plans to split into success and failure events. eg TASK/SUCCESS, TASK/FAILURE etc etc.

— Reply to this email directly or view it on GitHubhttps://github.com/Flipkart/phantom/pull/1#issuecomment-27367581 .

-Shashwat ::Flipkart::

amanhigh commented 11 years ago

Hi shashwata, Thanks for taking out time to review the code. I have removed enums and replaced them as String source, also changed ServiceProxyEvent variables scope to private. As consumers are implemented i will see how can we do better splitting of events.

Regards, Aman

shashwata commented 11 years ago

Thanks Amanpreet. On Oct 30, 2013 5:51 PM, "Amanpreet Singh" notifications@github.com wrote:

Hi shashwata, Thanks for taking out time to review the code. I have removed enums and replaced them as String source, also changed ServiceProxyEvent variables scope to private. As consumers are implemented i will see how can we do better splitting of events.

Regards, Aman

— Reply to this email directly or view it on GitHubhttps://github.com/Flipkart/phantom/pull/1#issuecomment-27384304 .