-
Notifications
You must be signed in to change notification settings - Fork 230
Fix event drop by fetching 3 pages with ETag optimization #317
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ | |
| ## Setup | ||
| ## | ||
|
|
||
| PAGE_LIMIT = 500 | ||
| PAGE_LIMIT = 100 | ||
|
|
||
| StatHat.config do |c| | ||
| c.ukey = ENV['STATHATKEY'] | ||
|
|
@@ -44,68 +44,122 @@ | |
|
|
||
| @latest = [] | ||
| @latest_key = lambda { |e| "#{e['id']}" } | ||
| @etags = {} # Track ETags for each page | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since tracking for each page, why not just use etag_1, etag_2 and etag_3? |
||
|
|
||
| process = Proc.new do | ||
| req = HttpRequest.new("https://api.github.com/events?per_page=#{PAGE_LIMIT}", { | ||
| :inactivity_timeout => 5, | ||
| :connect_timeout => 5 | ||
| }).get({ | ||
| # First, probe page 1 with conditional GET | ||
| req1 = HttpRequest.new("https://api.github.com/events?per_page=#{PAGE_LIMIT}&page=1", { | ||
| :inactivity_timeout => 5, | ||
| :connect_timeout => 5 | ||
| }).get({ | ||
| :head => { | ||
| 'user-agent' => 'gharchive.org', | ||
| 'Authorization' => 'token ' + ENV['GITHUB_TOKEN'] | ||
| } | ||
| 'Authorization' => 'token ' + ENV['GITHUB_TOKEN'], | ||
| 'If-None-Match' => @etags[1] | ||
| }.compact | ||
| }) | ||
|
|
||
| req.callback do | ||
| req1.callback do | ||
| begin | ||
| latest = Yajl::Parser.parse(req.response) | ||
| urls = latest.collect(&@latest_key) | ||
| new_events = latest.reject {|e| @latest.include? @latest_key.call(e)} | ||
|
|
||
| @latest = urls | ||
|
|
||
| # Determine archive filename based on current time, before processing events | ||
| current_processing_time = Time.now | ||
| timestamp = current_processing_time.strftime('%Y-%m-%d-%-k') | ||
| archive = "data/#{timestamp}.json" | ||
|
|
||
| # Open or rotate file based on the current time's archive path | ||
| if @file.nil? || (archive != @file.to_path) | ||
| if !@file.nil? | ||
| @log.info "Rotating archive. Current: #{@file.to_path}, New: #{archive}" | ||
| @file.close | ||
| end | ||
| @file = File.new(archive, "a+") | ||
| # If page 1 hasn't changed (304 Not Modified), skip this cycle | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a fact that if the 1st page is not modified, then 2 and 3 are not either? |
||
| if req1.response_header.status == 304 | ||
| @log.debug "Page 1 not modified, skipping" | ||
| EM.add_timer(0.2, &process) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the rationale for 0.2 seconds? |
||
| return | ||
| end | ||
|
|
||
| new_events.each do |event| | ||
| @file.puts(Yajl::Encoder.encode(Obfuscate.email(event))) | ||
| end | ||
| # Page 1 changed, update ETag and fetch pages 2 & 3 | ||
| @etags[1] = req1.response_header.etag | ||
|
|
||
| # Fetch pages 2 and 3 in parallel (GitHub only provides up to 300 events) | ||
| multi = EM::MultiRequest.new | ||
|
|
||
| req2 = HttpRequest.new("https://api.github.com/events?per_page=#{PAGE_LIMIT}&page=2", { | ||
| :inactivity_timeout => 5, | ||
| :connect_timeout => 5 | ||
| }).get({ | ||
| :head => { | ||
| 'user-agent' => 'gharchive.org', | ||
| 'Authorization' => 'token ' + ENV['GITHUB_TOKEN'], | ||
| 'If-None-Match' => @etags[2] | ||
| }.compact | ||
| }) | ||
|
|
||
| req3 = HttpRequest.new("https://api.github.com/events?per_page=#{PAGE_LIMIT}&page=3", { | ||
| :inactivity_timeout => 5, | ||
| :connect_timeout => 5 | ||
| }).get({ | ||
| :head => { | ||
| 'user-agent' => 'gharchive.org', | ||
| 'Authorization' => 'token ' + ENV['GITHUB_TOKEN'], | ||
| 'If-None-Match' => @etags[3] | ||
| }.compact | ||
| }) | ||
|
|
||
| multi.add(:page2, req2) | ||
| multi.add(:page3, req3) | ||
|
|
||
| multi.callback do | ||
| # Update ETags | ||
| @etags[2] = req2.response_header.etag if req2.response_header.status == 200 | ||
| @etags[3] = req3.response_header.etag if req3.response_header.status == 200 | ||
|
|
||
| # Parse all responses | ||
| page1_events = Yajl::Parser.parse(req1.response) | ||
| page2_events = req2.response_header.status == 200 ? Yajl::Parser.parse(req2.response) : [] | ||
| page3_events = req3.response_header.status == 200 ? Yajl::Parser.parse(req3.response) : [] | ||
|
|
||
| # Merge all events from the 3 pages (GitHub's max is 300 events) | ||
| latest = page1_events + page2_events + page3_events | ||
| urls = latest.collect(&@latest_key) | ||
| new_events = latest.reject {|e| @latest.include? @latest_key.call(e)} | ||
|
|
||
| @latest = urls | ||
|
|
||
| # Determine archive filename based on current time, before processing events | ||
| current_processing_time = Time.now | ||
| timestamp = current_processing_time.strftime('%Y-%m-%d-%-k') | ||
| archive = "data/#{timestamp}.json" | ||
|
|
||
| # Open or rotate file based on the current time's archive path | ||
| if @file.nil? || (archive != @file.to_path) | ||
| if !@file.nil? | ||
| @log.info "Rotating archive. Current: #{@file.to_path}, New: #{archive}" | ||
| @file.close | ||
| end | ||
| @file = File.new(archive, "a+") | ||
| end | ||
|
|
||
| remaining = req.response_header.raw['X-RateLimit-Remaining'] | ||
| reset = Time.at(req.response_header.raw['X-RateLimit-Reset'].to_i) | ||
| @log.info "Found #{new_events.size} new events: #{new_events.collect(&@latest_key)}, API: #{remaining}, reset: #{reset}" | ||
| new_events.each do |event| | ||
| @file.puts(Yajl::Encoder.encode(Obfuscate.email(event))) | ||
| end | ||
|
|
||
| if new_events.size >= PAGE_LIMIT | ||
| @log.info "Missed records.." | ||
| end | ||
| remaining = req1.response_header.raw['X-RateLimit-Remaining'] | ||
| reset = Time.at(req1.response_header.raw['X-RateLimit-Reset'].to_i) | ||
| @log.info "Found #{new_events.size} new events (page1: #{page1_events.size}, page2: #{page2_events.size}, page3: #{page3_events.size}), API: #{remaining}, reset: #{reset}" | ||
|
|
||
| StatHat.new.ez_count('Github Events', new_events.size) | ||
| if new_events.size >= (PAGE_LIMIT * 3) | ||
| @log.warn "Potentially missed records - got #{new_events.size} new events (at GitHub's 300 event limit)" | ||
| end | ||
|
|
||
| StatHat.new.ez_count('Github Events', new_events.size) | ||
|
|
||
| EM.add_timer(0.2, &process) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the rationale for 0.2 secs? |
||
| end | ||
|
|
||
| rescue Exception => e | ||
| @log.error "Failed to process response" | ||
| @log.error "Response: #{req.response}" | ||
| @log.error "Response headers: #{req.response_header}" | ||
| @log.error "Response page 1: #{req1.response}" | ||
| @log.error "Response headers: #{req1.response_header}" | ||
| @log.error "Processing exception: #{e}, #{e.backtrace.first(5)}" | ||
| ensure | ||
| EM.add_timer(0.75, &process) | ||
| end | ||
| end | ||
|
|
||
| req.errback do | ||
| @log.error "Error: #{req.response_header.status}, \ | ||
| header: #{req.response_header}, \ | ||
| response: #{req.response}" | ||
| req1.errback do | ||
| @log.error "Error fetching page 1: #{req1.response_header.status}, \ | ||
| header: #{req1.response_header}, \ | ||
| response: #{req1.response}" | ||
|
|
||
| EM.add_timer(0.75, &process) | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd keep these changes in a separate PR.