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

Creates batch adapter #58

Closed
wants to merge 8 commits into from
Closed

Creates batch adapter #58

wants to merge 8 commits into from

Conversation

tpitale
Copy link
Owner

@tpitale tpitale commented Dec 2, 2015

  • Size-bounded queue using ruby stdlib SizedQueue for thread-safe queueing

TODO:

  • test what net/http, faraday adapters do when given an array (must match batch body for GA)
  • discuss clear behavior
  • usage documentation
  • update our usage of net/http to support array or string

Resolves #57

@tpitale
Copy link
Owner Author

tpitale commented Dec 2, 2015

@wjordan: Here's a quick spike. One issue (seemingly present in all the queueing/buffer code I looked at) is: how/when to trigger the "final" flushing of the buffer.

I've added a method called clear on the adapter, I could delegate to this from the tracker, but the application using this would have to somehow know when to call it. One example, a rails app, is probably not aware when the server crashes/quits. Events in this case would remain un-sent to GA. Seems less than ideal.

@wjordan
Copy link

wjordan commented Dec 2, 2015

@tpitale Awesome progress! This looks feasible. Flushing a partially-filled buffer can be handled a few different ways:

  1. Async flush operation the application can manually invoke to flush a partially-filled batch, e.g. at the end of a per-request ensure block (ruby-stud :force option)
  2. Configurable interval timer that automatically flushes a partially-filled buffer after a maximum delay
  3. Blocking flush operation the application can manually invoke to definitively flush the remaining buffered items e.g. on teardown, ensure block, etc. (ruby-stud :final option)

the last one is already covered by the existing clear method. For 1-2, we would need clear to support a partial flush without killing flushing_thread. This could be added to this PR by pushing a Null Object to the queue (to wake up the waiting flushing_thread), maybe something like:

FLUSH_OBJECT = Object.new.freeze
def clear(final=false)
  @queue << FLUSH_OBJECT
  if final
    @flushing_thread.kill
    @flushing_thread.join
    flush(true)
  end
end

def flush(final=false)
  param_array = []
  while param_array.size < @size && (object = @queue.deq(final)) != FLUSH_OBJECT
    param_array << object
  end
  rescue ThreadError
  ensure
    @adapter.post(param_array)
  end
end

@tpitale
Copy link
Owner Author

tpitale commented Dec 3, 2015

Okay, so you're thinking the use case would be to flush it at the end of a request, so that N events would get queued up and make a minimal number of calls to Google.

I think I can make some tweaks to make that work.

@sebbean
Copy link

sebbean commented Sep 29, 2020

@tpitale i commend how long you've been working on this thing! it has really help us out in our usecase (analytics for an iframe embed interactive widget, frontend analytics is being blocked)

@tpitale
Copy link
Owner Author

tpitale commented Nov 27, 2020

@wjordan @sebbean Are either of you using this? It has been sitting for a long time, and I'm not sure if anyone really needs it anymore …?

@adapter = adapter.new(Staccato.ga_batch_uri)

@queue = SizedQueue.new(@size)
@flushing_thread = Thread.new { loop { perform_flush; sleep(flush_timeout) } }
Copy link
Owner Author

Choose a reason for hiding this comment

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

@bots-business I could change this PR so that no flushing_thread is created if the flush_timeout is explicitly set to nil. This way the tracker and adapter are still thread-safe, but no additional thread is created to perform auto-flushing of the queue.

@tpitale
Copy link
Owner Author

tpitale commented May 11, 2023

Closing this as old and not sure it's necessary yet in V4.

@tpitale tpitale closed this May 11, 2023
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.

batching
3 participants