Skip to content
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

Feature/distributed state registry #312

Merged
merged 4 commits into from
Aug 14, 2016

Conversation

WolframHempel
Copy link
Member

The distributed state registry on its own. I think this might be useful for a couple of cases, e.g. keeping remote server urls for failover, keeping track of listeners etc.

There is a todo left which regards cluster-presence. In order to make sure that all nodes within a cluster are still present, we might want to implement a global registry, rather than many local ones. Let's talk about this on Monday.

@coveralls
Copy link

coveralls commented Jul 31, 2016

Coverage Status

Coverage decreased (-0.3%) to 96.24% when pulling 5335f8e on feature/distributed-state-registry into b026f54 on master.

@ronag
Copy link

ronag commented Jul 31, 2016

Just a question. Wouldn't it be better to have a state registry connector instead and use existing services, e.g. etcd, zookeeper or consul?

@WolframHempel
Copy link
Member Author

We feel that it's important for deepstream going forward to be a truly distributed system without master/slave relationships or a centralized state registry (if we needed that, we could have just written to the cache)

@ronag
Copy link

ronag commented Jul 31, 2016

@WolframHempel: All of the tree software I listed are distributed key-value stores... I'm not saying it's a bad idea do have it native in ds. Just wondering if the option has been considered. Implementing a consensus algorithm is no small task...

}

return checkSum;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a very naive checksum. Sure it's good enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate on the problem you see?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating input values that return the same checksum is rather trivial... e.g. "ab" would have the same checksum as "ba".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, but we're using the sum of the checksums to get a rough impression of state-consistency. This happens every time something is added to the registry, so its a trade off between performance and uniqueness. Happy to go for something more elaborate though if you've got an algorithm in mind that doesn't come with significant performance overhead

Copy link

@ronag ronag Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just doing something like:

checkSum = ( ( checkSum << 5 ) - checkSum ) + name.charCodeAt( i );
checkSum = checkSum & checkSum; // Convert to 32bit integer

Would be a huge improvement.

http://werxltd.com/wp/2010/05/13/javascript-implementation-of-javas-string-hashcode-method/

Copy link
Member Author

@WolframHempel WolframHempel Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me 👍 (not too sure if we need the added checkSum & checkSum though)

@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage decreased (-0.3%) to 96.24% when pulling 42f92df on feature/distributed-state-registry into b026f54 on master.

@yasserf
Copy link
Contributor

yasserf commented Aug 3, 2016

@ronag there was a discussion about whether we can leverage other technologies ability for that. Problem is however that we have a flexible set of plugins and we don't want to suddenly tie ourselves down.

We are also working on a lightweight peer2peer messaging layer incase a distributed message system is considered overkill, which wouldn't work without native implementation. Hope that clears it up!

@yasserf yasserf merged commit 284e0ed into master Aug 14, 2016
@yasserf yasserf deleted the feature/distributed-state-registry branch August 14, 2016 10:22
jaime-ez pushed a commit to jaime-ez/deepstream.io that referenced this pull request Feb 20, 2024
Setting anonymous record with same name discards and resubscribes
the same record, it should just ignore it

Fixes deepstreamIO#312
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants