-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Batching/scheduling #278
Batching/scheduling #278
Conversation
f1eae52
to
be178d6
Compare
943a109
to
6672fa8
Compare
Can you rebase this on top of master now that the other PR is merged? It looks like they have some overlap now. |
I assume the reason it's done as part of the network interface is because when we build batching at the transport level, we'll want to have a special network interface for that transport? |
Yes, the idea is that different servers may not all support the same kind of batching. There's a "batcher" in the client that batches up requests and sends them to the network interface at the same time, but it's up to the network interface to decide whether it sends the queries batched or not. Scheduling for polling queries is going to be another separate module, which just aligns polling intervals. |
6672fa8
to
a640c27
Compare
|
||
public start(pollInterval: Number) { | ||
this.pollInterval = pollInterval; | ||
this.pollTimer = setInterval(() => { |
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.
Does this setInterval
even use the pollInterval
? Or am I missing something?
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.
good catch - I'm not sure what happened there
Fixed @stubailo |
df6c746
to
d6ce369
Compare
d6ce369
to
33669b6
Compare
…ked network interface
…only polling queries
4461e86
to
9bd9996
Compare
queryManager: this, | ||
}); | ||
this.batcher = new QueryBatcher({ | ||
//we batch if the network interface supports batching if user has not specified |
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.
This comment is not correct
Query docs: PropTypes is now a separate package
…ling Batching/scheduling
This PR (originally part of PR #266) implements the logic that allows for queries to be batched together automatically. It is still WIP and I will now be implementing the changes as discussed with @helfer.
TODO: