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

Feature coalescing support #23

Merged
merged 9 commits into from
Apr 6, 2016
Merged

Conversation

kylehuntsman
Copy link
Contributor

Added coalescing support to save resources. Coalescing time window can be configured in the config in the "coalescingPeriod" property. Duplicate requests that are fired while a job is building will be thrown away and redirected to the running job. Duplicate requests that are fired during the coalescing period will also be thrown away and redirected to the original job.

@kylehuntsman
Copy link
Contributor Author

Hold off on review until tests have been made.

… that hadn't ended yet. Added test for coalescing.
@kylehuntsman
Copy link
Contributor Author

Added integration test.

@@ -14,14 +14,13 @@ module.exports = Datastore;
*/
function Datastore() {
// TODO: Switch this to something that can manage the size, evict old, etc. LRU maybe?
this.cache = {};
this.cache = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Clarification: Any reason this needs to be an array instead of object ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. That was never reverted back. It should be an object.

@sukrit007
Copy link
Contributor

@FunnyGopher Looks like there are some conflicts (possibly due to other branch that got merged with develop). Could you merge develop onto this branch and resolve conflicts ?

@kylehuntsman
Copy link
Contributor Author

Thanks for pointing that out. I need to go back and revert some of the code.

* @returns [] of jobs
*/
Factory.prototype.getJobs = function getJobs() {
var datastore = this.jobs.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Possibly use https://lodash.com/docs#values

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 92.06% when pulling 173ac68 on feature_coalescing-support into 55bc2af on develop.

@sukrit007 sukrit007 merged commit b15e443 into develop Apr 6, 2016
@sukrit007 sukrit007 deleted the feature_coalescing-support branch April 6, 2016 02:44
@sukrit007 sukrit007 restored the feature_coalescing-support branch April 6, 2016 03:02
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.

None yet

3 participants