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

Allow PartitionSource to limit the number of open files #1078

Merged
merged 3 commits into from
Oct 24, 2014

Conversation

matt-martin
Copy link
Contributor

Without this new parameter, partitioned sources always defaulted to a limit of 300 open files. This change should maintain the same default behavior while allowing a little more flexibility.

@@ -35,7 +35,7 @@ import cascading.tuple.Fields
/**
* This is a base class for partition-based output sources
*/
abstract class PartitionSource extends SchemedSource {
abstract class PartitionSource(val openWritesThreshold: Int = -1) extends SchemedSource {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be an option, not defaulting to -1

…s can specify a threshold less than or equal to zero. There doesn't seem to be any error checking in Cascading that forbids this, but the end result is that it's as if they passed in a threshold of 1.
@matt-martin
Copy link
Contributor Author

Thanks @ianoc! I've incorporated your changes with my latest commit.

@ianoc
Copy link
Collaborator

ianoc commented Oct 23, 2014

Looks like the tests fail with this at the moment

@matt-martin
Copy link
Contributor Author

Oops, my bad @ianoc. Should be fixed now...

@matt-martin
Copy link
Contributor Author

@ianoc Build/tests just completed successfully. See: https://travis-ci.org/twitter/scalding/builds/38880158

@ianoc
Copy link
Collaborator

ianoc commented Oct 24, 2014

Looks good, thanks for the contribution!

ianoc added a commit that referenced this pull request Oct 24, 2014
Allow PartitionSource to limit the number of open files
@ianoc ianoc merged commit b67c947 into twitter:develop Oct 24, 2014
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

2 participants