-
Notifications
You must be signed in to change notification settings - Fork 24
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
HPCC4J-639 Add concurrent connection startup limit #760
Conversation
Jira Issue: https://hpccsystems.atlassian.net/browse/HPCC4J-639 Jirabot Action Result: |
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 seems fine, but it appears to contain changes from the atomic increment pr, does this need to be rebased?
@@ -622,12 +661,14 @@ private static Options getReadTestOptions() | |||
options.addOption("pass", true, "Specifies the password used to connect. Defaults to null."); | |||
options.addOption("num_threads", true, "Specifies the number of parallel to use to perform operations."); | |||
options.addOption("access_expiry_seconds", true, "Access token expiration seconds."); | |||
options.addOption("initial_read_size", true, "The size of the initial read request in KB sent to the rowservice."); |
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.
perhaps a comment on why users would be interested in setting this value
while (fileReader.hasNext()) | ||
{ | ||
HPCCRecord record = fileReader.next(); | ||
context.getCurrentOperation().recordsRead.incrementAndGet(); | ||
recCount++; |
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.
is this from a separate PR?
try | ||
{ | ||
Thread.sleep(1); | ||
} catch (InterruptedException e) {} // We don't care about waking early |
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.
minor format issue, missing new line
@@ -857,6 +923,7 @@ public void startBlockingFetchRequest(List<Long> fetchOffsets) throws Exception | |||
{ | |||
finishFetch(); | |||
} | |||
|
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.
minor extra new line.
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.
approved just a couple of minor formatting issues
- Added a limit to the number of connections that can be started simultaneously - Reduced the size of the initial read request - Exposed parameters in FileUtility Signed-off-by: James McMullan [email protected]
Jirabot Action Result: |
Signed-off-by: James McMullan [email protected]
Type of change:
Checklist:
Testing: