Skip to content

RUBY-278 - Remove profile inheritance logic; use system defaults inst… #217

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 7, 2016
Merged

RUBY-278 - Remove profile inheritance logic; use system defaults inst… #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions features/basics/execution_profiles.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ Feature: Execution profiles
"""
There are 2 execution profiles in the cluster:

Name: default
Load_balancing_policy: Cassandra::LoadBalancing::Policies::TokenAware
Retry policy: Cassandra::Retry::Policies::Default
Consistency: local_one
Timeout: 12

Name: my_profile
Load_balancing_policy: Cassandra::LoadBalancing::Policies::RoundRobin
Retry policy: Cassandra::Retry::Policies::DowngradingConsistency
Consistency: all
Timeout: 32

Name: default
Load_balancing_policy: Cassandra::LoadBalancing::Policies::TokenAware
Retry policy: Cassandra::Retry::Policies::Default
Consistency: local_one
Timeout: 12
"""

Scenario: Configure different load balancing policies with profiles
Expand Down
11 changes: 7 additions & 4 deletions integration/session_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,9 +910,9 @@ def test_can_retrieve_cluster_options
#
# test_can_use_execution_profiles tests that execution profiles can be created and used in session execution. It first
# creates two execution profiles: one with all options specified and another with no options specified. It then
# creates a cluster with these two profiles and verifies their options. The 2nd execution profile should be equivalent
# to the :default execution profile as any missing options are copied over. It then executes a simple query using
# the execution profiles and verifies that the execution info shows their use.
# creates a cluster with these two profiles and verifies their options. The 2nd execution profile should use
# system defaults for attributes. It then executes a simple query using the execution profiles and verifies that the
# execution info shows their use.
#
# @since 3.1.0
# @jira_ticket RUBY-256
Expand All @@ -939,7 +939,10 @@ def test_can_use_execution_profiles
assert_equal profile_1, execution_profile_1

execution_profile_2 = cluster.execution_profiles[:profile_2]
assert_equal cluster.execution_profiles[:default], execution_profile_2
assert_equal cluster.execution_profiles[:default].timeout, execution_profile_2.timeout
assert_equal cluster.execution_profiles[:default].consistency, execution_profile_2.consistency
assert_instance_of Cassandra::Retry::Policies::Default, execution_profile_2.retry_policy
assert_instance_of Cassandra::LoadBalancing::Policies::TokenAware, execution_profile_2.load_balancing_policy

session = cluster.connect
exec_options = session.execute('select * from system.local').execution_info.options
Expand Down
61 changes: 17 additions & 44 deletions lib/cassandra/execution/profile.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,44 +43,37 @@ class Profile
attr_reader :timeout

# @private
attr_accessor :parent_name
DEFAULT_OPTIONS = { consistency: :local_one, timeout: 12 }.freeze

# @private
DEFAULT_OPTIONS = {load_balancing_policy: :unspecified,
retry_policy: :unspecified,
consistency: :unspecified,
timeout: :unspecified}.freeze

# @private
DEFAULT_PARENT_NAME = :default

# @param options [Hash] hash of attributes. Unspecified attributes or attributes with nil values effectively
# fall back to the attributes in the default execution profile.
# @option options [Numeric] :timeout (:unspecified) Request execution timeout in
# @param options [Hash] hash of attributes. Unspecified attributes fall back to system defaults.
# @option options [Numeric] :timeout (12) Request execution timeout in
# seconds. Setting value to `nil` will remove request timeout.
# @option options [Cassandra::LoadBalancing::Policy] :load_balancing_policy (:unspecified) Load-balancing policy
# @option options [Cassandra::LoadBalancing::Policy] :load_balancing_policy (
# LoadBalancing::Policies::TokenAware(LoadBalancing::Policies::DCAwareRoundRobin)) Load-balancing policy
# that determines which node will run the next statement.
# @option options [Cassandra::Retry::Policy] :retry_policy (:unspecified) Retry policy that determines how
# request retries should be handled for different failure modes.
# @option options [Symbol] :consistency (:unspecified) Consistency level with which to run statements. Must be one
# @option options [Cassandra::Retry::Policy] :retry_policy (Retry::Policies::Default) Retry policy that
# determines how request retries should be handled for different failure modes.
# @option options [Symbol] :consistency (:local_one) Consistency level with which to run statements. Must be one
# of {Cassandra::CONSISTENCIES}.
def initialize(options = {})
validate(options)
options = DEFAULT_OPTIONS.merge(options)
@load_balancing_policy = options[:load_balancing_policy]
@retry_policy = options[:retry_policy]
@load_balancing_policy = options[:load_balancing_policy] ||
LoadBalancing::Policies::TokenAware.new(
LoadBalancing::Policies::DCAwareRoundRobin.new,
true)
@retry_policy = options[:retry_policy] || Retry::Policies::Default.new
@consistency = options[:consistency]
@timeout = options[:timeout]
@parent_name = DEFAULT_PARENT_NAME
end

# @private
def to_h
{
load_balancing_policy: @load_balancing_policy,
retry_policy: @retry_policy,
consistency: @consistency,
timeout: @timeout
load_balancing_policy: @load_balancing_policy,
retry_policy: @retry_policy,
consistency: @consistency,
timeout: @timeout
}
end

Expand Down Expand Up @@ -152,26 +145,6 @@ def validate(options)
"#{consistency.inspect} given")
end
end

# @private
def merge_from(parent_profile)
return self if well_formed?

parent_hash = parent_profile.to_h
self_hash = to_h
self_hash.each do |key, value|
self_hash[key] = parent_hash[key] if value == :unspecified
end
Profile.new(self_hash)
end

# @private
def well_formed?
!@load_balancing_policy.nil? && @load_balancing_policy != :unspecified &&
!@retry_policy.nil? && @retry_policy != :unspecified &&
!@consistency.nil? && @consistency != :unspecified &&
@timeout != :unspecified
end
end
end
end
52 changes: 6 additions & 46 deletions lib/cassandra/execution/profile_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,16 @@ class ProfileManager

def initialize(default_profile, profiles)
# default_profile is the default profile that we constructed internally. However, the user can override it
# with their own :default profile, which may not be fully specified. See if we have such a profile and merge
# in the "system defaults" from the profile we generated.
# with their own :default profile. If that happens, ignore the internally generated default profile.

custom_default = profiles.delete(:default)
unless custom_default.nil?
default_profile = custom_default.merge_from(default_profile)
end

# Walk through the profiles and fill them out with attributes from the default profile when they're not
# set. Also, save off all of the load-balancing policies for easy access.
profiles[:default] = default_profile unless profiles.key?(:default)

# Save off all of the load-balancing policies for easy access.
@load_balancing_policies = Set.new
@load_balancing_policies << default_profile.load_balancing_policy
@profiles = {default: default_profile}

@unready_profiles = {}

profiles.each do |name, profile|
add_profile(name, profile)
@load_balancing_policies << profile.load_balancing_policy
end
@profiles = profiles
end

def default_profile
Expand All @@ -67,44 +57,14 @@ def distance(host)
# NOTE: It's only safe to call add_profile when setting up the cluster object. In particular,
# this is only ok before calling Driver#connect.
def add_profile(name, profile)
if !profile.well_formed? && @profiles.key?(profile.parent_name)
# This profile is ready to inherit attributes from its parent.
profile = profile.merge_from(@profiles[profile.parent_name])
end
if profile.well_formed?
make_available(name, profile)
did_add = true
while did_add && !@unready_profiles.empty?
did_add = false
@unready_profiles.dup.each do |name, profile|
did_add = hydrate_profile(name, profile)
end
end
else
# This profile isn't ready to inherit its parent attributes yet
@unready_profiles[name] = profile
end
end

def make_available(name, profile)
@profiles[name] = profile
@load_balancing_policies << profile.load_balancing_policy
@unready_profiles.delete(name)
end

def hydrate_profile(name, profile)
if @profiles.key?(profile.parent_name)
profile = profile.merge_from(@profiles[profile.parent_name])
make_available(name, profile)
return true
end
false
end

# @private
def inspect
"#<#{self.class.name}:0x#{object_id.to_s(16)} " \
"profiles=#{@profiles.inspect}>"
"profiles=#{@profiles.inspect}>"
end
end
end
Expand Down
34 changes: 5 additions & 29 deletions spec/cassandra/execution/profile_manager_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,15 @@ module Execution
let(:profile1) { Profile.new(load_balancing_policy: lbp1) }
let(:profile2) { Profile.new(load_balancing_policy: lbp2) }
let(:profile3) { Profile.new(load_balancing_policy: lbp3) }
let(:profile4) { Profile.new }
let(:profile5) { Profile.new(load_balancing_policy: lbp1) }
let(:profile6) {
# This profile's parent is *not* the default, but rather one of the other profiles.
p = Profile.new
p.parent_name = :p3
p
}
let(:default_profile) {
Profile.new(load_balancing_policy: lbp1, retry_policy: retry_policy, consistency: :quorum, timeout: 12)
}
let(:subject) {
ProfileManager.new(default_profile, {p6: profile6, p1: profile1, p2: profile2, p3: profile3, p4: profile4, p5: profile5})
ProfileManager.new(default_profile, {p1: profile1, p2: profile2, p3: profile3, p5: profile5})
}
let(:subject_with_custom_default) {
ProfileManager.new(default_profile, {default: profile2, p4: profile4})
ProfileManager.new(default_profile, {default: profile2})
}

before do
Expand All @@ -65,30 +58,13 @@ module Execution
allow(retry_policy).to receive(:unavailable)
end

it 'should fill out profiles with values from default profile' do
expect(subject.profiles[:p1]).to eq(Profile.new(load_balancing_policy: lbp1, retry_policy: retry_policy,
consistency: :quorum, timeout: 12))
expect(subject.profiles[:p2]).to eq(Profile.new(load_balancing_policy: lbp2, retry_policy: retry_policy,
consistency: :quorum, timeout: 12))
expect(subject.profiles[:p3]).to eq(Profile.new(load_balancing_policy: lbp3, retry_policy: retry_policy,
consistency: :quorum, timeout: 12))
expect(subject.profiles[:p4]).to eq(Profile.new(load_balancing_policy: lbp1, retry_policy: retry_policy,
consistency: :quorum, timeout: 12))
expect(subject.profiles[:p5]).to eq(Profile.new(load_balancing_policy: lbp1, retry_policy: retry_policy,
consistency: :quorum, timeout: 12))
expect(subject.profiles[:p6]).to eq(subject.profiles[:p3])
end

it 'should respect custom default profile' do
expect(subject_with_custom_default.profiles[:p4]).to eq(Profile.new(load_balancing_policy: lbp2,
retry_policy: retry_policy,
consistency: :quorum,
timeout: 12))
expect(subject_with_custom_default.profiles[:default]).to be(profile2)
end

it 'should return unique list of lbps' do
lbps = Set.new([lbp1, lbp2, lbp3])
expect(subject.load_balancing_policies).to eq(lbps)
expect(subject.load_balancing_policies.size).to eq(3)
expect(subject.load_balancing_policies.include?(lbp1)).to eq(true)
end

context :distance do
Expand Down
43 changes: 8 additions & 35 deletions spec/cassandra/execution/profile_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,45 +74,18 @@ module Execution
end
expect { Profile.new(consistency: 'foo') }.to raise_error(ArgumentError)
end
end

context :merge_from do
let(:default_profile) { Profile.new(load_balancing_policy: lbp, retry_policy: retry_policy, consistency: :one,
timeout: 10)}
let(:profile) {
Profile.new(load_balancing_policy: lbp2, retry_policy: retry_policy2, consistency: :quorum, timeout: 23)
}
let(:empty_profile) { Profile.new }

it 'should accept all attributes from parent profile if it has no attributes itself' do
expect(Profile.new.merge_from(default_profile)).to eq(default_profile)
end

it 'should ignore all attributes from parent profile if it is fully specified itself' do
expect(profile.merge_from(default_profile)).to be(profile)
it 'should fall back to system defaults for unspecified attributes' do
p = Profile.new
expect(p.load_balancing_policy).to be_a(LoadBalancing::Policies::TokenAware)
expect(p.retry_policy).to be_a(Retry::Policies::Default)
expect(p.timeout).to eq(12)
expect(p.consistency).to be(:local_one)
end

it 'should respect nil timeout in parent' do
parent_profile = Profile.new(load_balancing_policy: lbp,
retry_policy: retry_policy,
consistency: :one,
timeout: nil)
expect(empty_profile.timeout).to eq(:unspecified)
expect(empty_profile.merge_from(parent_profile).timeout).to be_nil
it 'should support nil timeout' do
expect(Profile.new(timeout: nil).timeout).to be_nil
end

it 'should preserve its nil timeout when parent timeout is not nil' do
profile = Profile.new(timeout: nil)
expect(profile.timeout).to be_nil
expect(profile.merge_from(default_profile).timeout).to be_nil
end
end

it 'should not be well-formed if it has unspecified attributes' do
expect(Profile.new(timeout: 7).well_formed?).to eq(false)
expect(Profile.new(load_balancing_policy: lbp2, timeout: 7).well_formed?).to eq(false)
expect(Profile.new(retry_policy: retry_policy, load_balancing_policy: lbp2, timeout: 7).well_formed?).to eq(false)
expect(Profile.new(consistency: :one, retry_policy: retry_policy, load_balancing_policy: lbp2, timeout: 7).well_formed?).to eq(true)
end
end
end
Expand Down