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

Process bundle batch in parallel #2905

Merged
merged 10 commits into from
Aug 21, 2021
Merged

Conversation

frankjtao
Copy link
Collaborator

No description provided.

@frankjtao frankjtao self-assigned this Aug 19, 2021
@codecov
Copy link

codecov bot commented Aug 20, 2021

Codecov Report

Merging #2905 (45b23ce) into master (7ee22a4) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2905      +/-   ##
============================================
- Coverage     82.58%   82.57%   -0.02%     
+ Complexity    19723    19720       -3     
============================================
  Files          1327     1327              
  Lines         70182    70228      +46     
  Branches      10736    10740       +4     
============================================
+ Hits          57959    57989      +30     
- Misses         8102     8115      +13     
- Partials       4121     4124       +3     
Impacted Files Coverage Δ
...ain/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java 88.97% <ø> (+0.23%) ⬆️
...a/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java 92.39% <ø> (ø)
.../ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java 87.15% <ø> (+0.43%) ⬆️
...hn/fhir/jpa/search/PersistedJpaBundleProvider.java 90.60% <ø> (+0.12%) ⬆️
.../uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java 92.69% <ø> (+0.03%) ⬆️
...n/fhir/jpa/config/HapiFhirHibernateJpaDialect.java 75.00% <0.00%> (-15.63%) ⬇️
.../uhn/fhir/jpa/dao/r4/FhirResourceDaoPatientR4.java 81.81% <0.00%> (-9.10%) ⬇️
...ir/jpa/dao/BaseHapiFhirResourceDaoObservation.java 76.19% <0.00%> (-4.77%) ⬇️
.../fhir/jpa/dao/predicate/querystack/QueryStack.java 68.33% <0.00%> (-3.34%) ⬇️
...ca/uhn/fhir/jpa/dao/tx/HapiTransactionService.java 71.87% <0.00%> (-3.13%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb0552d...45b23ce. Read the comment docs.

Copy link
Contributor

@michaelabuckley michaelabuckley left a comment

Choose a reason for hiding this comment

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

Looks good.

//myExecutor.setMaxPoolSize(1);
myExecutor.setCorePoolSize(myDaoConfig.getBundleBatchPoolSize());
myExecutor.setMaxPoolSize(myDaoConfig.getBundleBatchMaxPoolSize());
myExecutor.setQueueCapacity(myDaoConfig.getBundleBatchQueueCapacity());
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the complexity of a queue? The main thread will wait for these to complete, so I wonder if we could delete this config.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will delete this config


myVersionAdapter.setResponseStatus(nextEntry, toStatusString(caughtEx.getException().getStatusCode()));
// waiting for all tasks to be completed
AsyncUtil.awaitLatchAndIgnoreInterrupt(completionLatch, 120L, TimeUnit.SECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we already have a request timeout configured somewhere? This feels too short for some customers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a connection timeout for 10000 milliseconds (10s). I can't find any other timeout value. The value here is 120s (2m). I can change it to 5m (300s).

@frankjtao frankjtao linked an issue Aug 20, 2021 that may be closed by this pull request
@frankjtao frankjtao merged commit 4c2ae51 into master Aug 21, 2021
@frankjtao frankjtao deleted the ft-batch-queries-in-parallel-2 branch August 21, 2021 01:14
@frankjtao frankjtao restored the ft-batch-queries-in-parallel-2 branch August 21, 2021 01:14
@frankjtao frankjtao deleted the ft-batch-queries-in-parallel-2 branch August 24, 2021 01:54
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.

Execute bundle searches in parallel
2 participants