Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

initial add of the Riak Core Connection Manager #284

Merged
merged 5 commits into from

5 participants

@metadave

This is Chris Tilt's work, I'm just moving files between repos.
see repl PR 212.

cc @buddhisthead @lordnull

NOTE Ranch has been added as a dependency for the connection manager tests to run.

@buddhisthead

Shouldn't this line be in the "riak_core_cluster.hrl"?

-type(cluster_finder_fun() :: fun(() -> {ok,node()} | {error, term()})).

I thought so too, but it appears in several spots in the conn mgr.

@buddhisthead

I'm glad you moved these functions here!

@buddhisthead

Dave, after more consideration, I believe that we should remove the functions around cluster_finder from the connection manager. It is currently used only by the test modules and I think it is non-used API. We should remove it now, before making it open source, because the connection manager should be devoid of cluster-related connections.

This functionality has been replaced by remote targets, registration of them, etc. The cluster_finder is no longer used. Please cut it all out of the connection_mgr and from the associated test.

src/riak_core_service_mgr.erl
((370 lines not shown))
+ [{{_HostProto,HostVersions},Rest}=_Matched | _DuplicatesIgnored] ->
+ ?TRACE(?debugFmt("choose_version: unsorted = ~p clientversions = ~p",
+ [_Matched, ClientVersions])),
+ CommonVers = [{CM,CN,HN} || {CM,CN} <- ClientVersions, {HM,HN} <- HostVersions, CM == HM],
+ ?TRACE(?debugFmt("common versions = ~p", [CommonVers])),
+ %% sort by major version, highest to lowest, and grab the top one.
+ case lists:reverse(lists:keysort(1,CommonVers)) of
+ [] ->
+ %% oops! No common major versions for Proto.
+ ?TRACE(?debugFmt("Failed to find a common major version for protocol: ~p",
+ [ClientProto])),
+ lager:error("Failed to find a common major version for protocol: ~p", [ClientProto]),
+ {error,protocol_version_not_supported,Rest};
+ [{Major,CN,HN}] ->
+ {ok, {ClientProto,Major,CN,HN},Rest};
+ [{Major,CN,HN}, _] ->
@lordnull Collaborator

This line should likly be:

[{Major,CN,HN} | _] ->

As is only 2 version permutations are allowed, anything else will get a case_clause error.

I found this while building a riak_test for cascading repl, which supports 2.0, 1.1, and 1.0 versions.

Ah! Good catch!
+1 to that change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@metadave

I added Ranch for the connection manager tests, however it looks like the dep doesn't work with R14B04 | R14B03
https://travis-ci.org/basho/riak_core/builds/5595971

@buddhisthead

Tests Pass.
+1

@metadave metadave was assigned
@metadave metadave merged commit dcf8e52 into master
@yrashk

Where can I learn what this change is intended for? Thanks!

It's the basis for the enhancements to replication in the EE product. It was written to provide a generally useful mechanism for inter-VM communication which may be useful for overhauling the handoff protocol or offloading large data transfers from disterl.

Anything that needs a socket and wants to negotiate protocol versions.

@yrashk

Any reason for using an old version of ranch?

Yes, it's pinned now for an internal project. Once we get things moving along, we'll update the dep.

@seancribbs seancribbs deleted the dip_conn_mgr branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 13, 2013
  1. initial add of the Riak Core Connection Manager

    Dave Parfitt authored
Commits on Mar 15, 2013
  1. @lordnull
Commits on Mar 18, 2013
  1. add connection manager tests from repl

    Dave Parfitt authored
  2. added ranch as a dep for connection manager tests

    Dave Parfitt authored
Something went wrong with that request. Please try again.