Skip to content

Commit 795329c

Browse files
sanzmaurosanzmauro
authored
Merge pull request #381 from splitio/segments-retries
Segments: add new logic for retries and logic with cdn bypassed
2 parents 7f7028b + cd683aa commit 795329c

11 files changed

Lines changed: 162 additions & 54 deletions

File tree

‎lib/splitclient-rb.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@
8585
require 'splitclient-rb/engine/models/label'
8686
require 'splitclient-rb/engine/models/treatment'
8787
require 'splitclient-rb/engine/auth_api_client'
88+
require 'splitclient-rb/engine/back_off'
8889
require 'splitclient-rb/engine/push_manager'
8990
require 'splitclient-rb/engine/sync_manager'
9091
require 'splitclient-rb/engine/synchronizer'
9192
require 'splitclient-rb/utilitites'
9293

93-
# SSE
94-
require 'splitclient-rb/sse/event_source/back_off'
94+
# SSE
9595
require 'splitclient-rb/sse/event_source/client'
9696
require 'splitclient-rb/sse/event_source/event_parser'
9797
require 'splitclient-rb/sse/event_source/event_types'

‎lib/splitclient-rb/engine/api/segments.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ def fetch_segments_by_names(names, fetch_options = { cache_control_headers: fals
1616

1717
names.each do |name|
1818
since = @segments_repository.get_change_number(name)
19+
1920
loop do
2021
segment = fetch_segment_changes(name, since, fetch_options)
2122
@segments_repository.add_to_segment(segment)

‎lib/splitclient-rb/engine/back_off.rb‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# frozen_string_literal: false
2+
3+
module SplitIoClient
4+
module Engine
5+
BACKOFF_MAX_ALLOWED = 1.8
6+
class BackOff
7+
def initialize(back_off_base, attempt = 0, max_allowed = BACKOFF_MAX_ALLOWED)
8+
@attempt = attempt
9+
@back_off_base = back_off_base
10+
@max_allowed = max_allowed
11+
end
12+
13+
def interval
14+
interval = 0
15+
interval = (@back_off_base * (2**@attempt)) if @attempt.positive?
16+
@attempt += 1
17+
18+
interval >= @max_allowed ? @max_allowed : interval
19+
end
20+
21+
def reset
22+
@attempt = 0
23+
end
24+
end
25+
end
26+
end

‎lib/splitclient-rb/engine/push_manager.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def initialize(config, sse_handler, api_key, telemetry_runtime_producer)
88
@sse_handler = sse_handler
99
@auth_api_client = AuthApiClient.new(@config, telemetry_runtime_producer)
1010
@api_key = api_key
11-
@back_off = SplitIoClient::SSE::EventSource::BackOff.new(@config.auth_retry_back_off_base, 1)
11+
@back_off = Engine::BackOff.new(@config.auth_retry_back_off_base, 1)
1212
@telemetry_runtime_producer = telemetry_runtime_producer
1313
end
1414

‎lib/splitclient-rb/engine/synchronizer.rb‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,32 +86,80 @@ def fetch_splits(target_change_number)
8686

8787
if result[:success]
8888
@segment_fetcher.fetch_segments_if_not_exists(result[:segment_names], true) unless result[:segment_names].empty?
89-
@config.logger.debug("Refresh completed bypassing the CDN in #{attempts} attempts.")
89+
@config.logger.debug("Refresh completed bypassing the CDN in #{attempts} attempts.") if @config.debug_enabled
9090
else
91-
@config.logger.debug("No changes fetched after #{attempts} attempts with CDN bypassed.")
91+
@config.logger.debug("No changes fetched after #{attempts} attempts with CDN bypassed.") if @config.debug_enabled
9292
end
9393
rescue StandardError => error
9494
@config.log_found_exception(__method__.to_s, error)
9595
end
9696

97-
def fetch_segment(name)
97+
def fetch_segment(name, target_change_number)
98+
return if target_change_number <= @segments_repository.get_change_number(name).to_i
99+
98100
fetch_options = { cache_control_headers: true, till: nil }
99-
@segment_fetcher.fetch_segment(name, fetch_options)
101+
result = attempt_segment_sync(name,
102+
target_change_number,
103+
fetch_options,
104+
@config.on_demand_fetch_max_retries,
105+
@config.on_demand_fetch_retry_delay_seconds,
106+
false)
107+
108+
attempts = @config.on_demand_fetch_max_retries - result[:remaining_attempts]
109+
if result[:success]
110+
@config.logger.debug("Segment #{name} refresh completed in #{attempts} attempts.") if @config.debug_enabled
111+
112+
return
113+
end
114+
115+
fetch_options = { cache_control_headers: true, till: target_change_number }
116+
result = attempt_segment_sync(name,
117+
target_change_number,
118+
fetch_options,
119+
ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES,
120+
nil,
121+
true)
122+
123+
attempts = @config.on_demand_fetch_max_retries - result[:remaining_attempts]
124+
if result[:success]
125+
@config.logger.debug("Segment #{name} refresh completed bypassing the CDN in #{attempts} attempts.") if @config.debug_enabled
126+
else
127+
@config.logger.debug("No changes fetched for segment #{name} after #{attempts} attempts with CDN bypassed.") if @config.debug_enabled
128+
end
129+
rescue StandardError => error
130+
@config.log_found_exception(__method__.to_s, error)
100131
end
101132

102133
private
103134

135+
def attempt_segment_sync(name, target_cn, fetch_options, max_retries, retry_delay_seconds, with_backoff)
136+
remaining_attempts = max_retries
137+
backoff = Engine::BackOff.new(ON_DEMAND_FETCH_BACKOFF_BASE_SECONDS, 0, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT_SECONDS) if with_backoff
138+
139+
loop do
140+
remaining_attempts -= 1
141+
142+
@segment_fetcher.fetch_segment(name, fetch_options)
143+
144+
return sync_result(true, remaining_attempts) if target_cn <= @segments_repository.get_change_number(name).to_i
145+
return sync_result(false, remaining_attempts) if remaining_attempts <= 0
146+
147+
delay = with_backoff ? backoff.interval : retry_delay_seconds
148+
sleep(delay)
149+
end
150+
end
151+
104152
def attempt_splits_sync(target_cn, fetch_options, max_retries, retry_delay_seconds, with_backoff)
105153
remaining_attempts = max_retries
106-
backoff = SSE::EventSource::BackOff.new(ON_DEMAND_FETCH_BACKOFF_BASE_SECONDS, 0, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT_SECONDS) if with_backoff
154+
backoff = Engine::BackOff.new(ON_DEMAND_FETCH_BACKOFF_BASE_SECONDS, 0, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT_SECONDS) if with_backoff
107155

108156
loop do
109157
remaining_attempts -= 1
110158

111159
segment_names = @split_fetcher.fetch_splits(fetch_options)
112160

113-
return split_sync_result(true, remaining_attempts, segment_names) if target_cn <= @splits_repository.get_change_number
114-
return split_sync_result(false, remaining_attempts, segment_names) if remaining_attempts <= 0
161+
return sync_result(true, remaining_attempts, segment_names) if target_cn <= @splits_repository.get_change_number
162+
return sync_result(false, remaining_attempts, segment_names) if remaining_attempts <= 0
115163

116164
delay = with_backoff ? backoff.interval : retry_delay_seconds
117165
sleep(delay)
@@ -141,7 +189,7 @@ def start_telemetry_sync_task
141189
Telemetry::SyncTask.new(@config, @telemetry_synchronizer).call
142190
end
143191

144-
def split_sync_result(success, remaining_attempts, segment_names)
192+
def sync_result(success, remaining_attempts, segment_names = nil)
145193
{ success: success, remaining_attempts: remaining_attempts, segment_names: segment_names }
146194
end
147195
end

‎lib/splitclient-rb/sse/event_source/back_off.rb‎

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎lib/splitclient-rb/sse/workers/segments_worker.rb‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
module SplitIoClient
44
module SSE
55
module Workers
6-
MAX_RETRIES_ALLOWED = 10
76
class SegmentsWorker
87
def initialize(synchronizer, config, segments_repository)
98
@synchronizer = synchronizer
@@ -52,11 +51,7 @@ def perform
5251
cn = item[:change_number]
5352
@config.logger.debug("SegmentsWorker change_number dequeue #{segment_name}, #{cn}")
5453

55-
attempt = 0
56-
while cn > @segments_repository.get_change_number(segment_name).to_i && attempt <= MAX_RETRIES_ALLOWED
57-
@synchronizer.fetch_segment(segment_name)
58-
attempt += 1
59-
end
54+
@synchronizer.fetch_segment(segment_name, cn)
6055
end
6156
end
6257

‎spec/engine/synchronizer_spec.rb‎

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@
9696
expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once
9797
end
9898

99-
it 'fetch_splits - ' do
100-
sync = subject.new(repositories, api_key, config, sdk_blocker, parameters)
99+
it 'fetch_splits - with CDN bypassed' do
101100
stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')
102101
.to_return(status: 200, body:
103102
'{
@@ -122,18 +121,85 @@
122121
"till": 1506703262921
123122
}')
124123

125-
sync.fetch_splits(1_506_703_262_920)
124+
synchronizer.fetch_splits(1_506_703_262_920)
126125

127126
expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once
128127
expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262918')).to have_been_made.times(9)
129128
expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262918&till=1506703262920')).to have_been_made.once
130129
end
131130

132131
it 'fetch_segment' do
133-
mock_segment_changes('segment3', segment3, '-1')
132+
stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1')
133+
.to_return(status: 200, body:
134+
'{
135+
"name": "segment3",
136+
"added": [],
137+
"removed": [],
138+
"since": -1,
139+
"till": 111333
140+
}')
141+
142+
stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111333')
143+
.to_return(status: 200, body:
144+
'{
145+
"name": "segment3",
146+
"added": [],
147+
"removed": [],
148+
"since": 111333,
149+
"till": 111333
150+
}')
151+
152+
synchronizer.fetch_segment('segment3', 111_222)
153+
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1')).to have_been_made.once
154+
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111333')).to have_been_made.once
155+
end
156+
157+
it 'fetch_segment - with CDN bypassed' do
158+
stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1')
159+
.to_return(status: 200, body:
160+
'{
161+
"name": "segment3",
162+
"added": [],
163+
"removed": [],
164+
"since": -1,
165+
"till": 111333
166+
}')
167+
168+
stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111333')
169+
.to_return(status: 200, body:
170+
'{
171+
"name": "segment3",
172+
"added": [],
173+
"removed": [],
174+
"since": 111333,
175+
"till": 111333
176+
}')
177+
178+
stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111333&till=111555')
179+
.to_return(status: 200, body:
180+
'{
181+
"name": "segment3",
182+
"added": [],
183+
"removed": [],
184+
"since": 111555,
185+
"till": 111555
186+
}')
187+
188+
stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111555&till=111555')
189+
.to_return(status: 200, body:
190+
'{
191+
"name": "segment3",
192+
"added": [],
193+
"removed": [],
194+
"since": 111555,
195+
"till": 111555
196+
}')
134197

135-
synchronizer.fetch_segment('segment3')
198+
synchronizer.fetch_segment('segment3', 111_555)
136199
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1')).to have_been_made.once
200+
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111333')).to have_been_made.times(10)
201+
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111333&till=111555')).to have_been_made.once
202+
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=111555&till=111555')).to have_been_made.once
137203
end
138204
end
139205

‎spec/sse/event_source/back_off_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
require 'spec_helper'
44
require 'http_server_mock'
55

6-
describe SplitIoClient::SSE::EventSource::BackOff do
7-
subject { SplitIoClient::SSE::EventSource::BackOff }
6+
describe SplitIoClient::Engine::BackOff do
7+
subject { SplitIoClient::Engine::BackOff }
88

99
let(:log) { StringIO.new }
1010

‎spec/sse/sse_handler_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@
212212

213213
expect(action_event).to eq(SplitIoClient::Constants::PUSH_CONNECTED)
214214
expect(sse_handler.sse_client.connected?).to eq(true)
215-
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment1?since=1470947453877')).to have_been_made.times(12)
215+
expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment1?since=1470947453877')).to have_been_made.times(11)
216216

217217
sse_handler.sse_client.close
218218

0 commit comments

Comments
 (0)