Skip to content

Enclose cursor operations in a with-yielding call. #64

Closed
wants to merge 2 commits into from

2 participants

@timclemons

The with-yielding call will produce a lazy-seq in a
separate thread that will generate content from the
cursor. The cursor will then be safely closed if the
seq is garbage collected or an error occurs.

This change was implemented to help facilitate the
safe use of tailable cursors.

See https://github.com/jpalmucci/clj-yield for more
details on the clj-yield library.

timclemons added some commits Nov 19, 2013
@timclemons timclemons Enclose cursor operations in a with-yielding call.
The with-yielding call will produce a lazy-seq in a
separate thread that will generate content from the
cursor.  The cursor will then be safely closed if the
seq is garbage collected or an error occurs.
d51c61d
@timclemons timclemons Fix indentation. f133fbe
@michaelklishin
Owner

Thanks. I'll need to take a closer at clj-yield to see if this is the kind of change I'd be comfortable with.

@timclemons

One implication is that each call to exec will now occur in a new thread. I wonder if it would be better to add an async option to with-collection instead and allow users to choose which behavior they want.

@michaelklishin
Owner

Yes, and this is a major step aside from how the rest of Monger works. I'd strongly prefer to have a new API element
(an option is one way) instead of changing it for everybody. If what you're after is tailable cursors, is there a way
this can be a new feature built on top of the current API?

@timclemons

I'll give it some thought on how to do this. However, I'll add that this would help with any query where it would be necessary to keep an open cursor. As is, the cursor will be always be closed on leaving the with-open call, making unavailable any query results not already pulled from the server.

@michaelklishin
Owner

That's true.

I'm generally fine with having an option if it does not lead to a major (say, over 10%) throughput drop for existing code
and we can come up with a descriptive name for it.

@michaelklishin
Owner

Thank you for working on this but I don't think the complexity it adds is worth including in the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.