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

New batch framework / Bulk Import #3387

Merged
merged 69 commits into from
Feb 27, 2022
Merged

Conversation

jamesagnew
Copy link
Collaborator

@jamesagnew jamesagnew commented Feb 13, 2022

Note: The QueryCount logic used to count SQL statements has changed so that batched statements are unrolled (e.g. if 3 SQL statements were identical and submitted as a batch that used to be counted as 1 and it's now counted as 3). A number of SQL statement counts have therefore increased in this ticket. I have confirmed every one of them and they are all not actually increases. This new way seems more accurate.

@lgtm-com
Copy link

lgtm-com bot commented Feb 13, 2022

This pull request introduces 2 alerts when merging 32de383 into 700221a - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2022

This pull request introduces 3 alerts when merging 9b6dbae into fb342cc - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null
  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2022

This pull request introduces 3 alerts when merging 19db798 into ae33cf8 - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null
  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 17, 2022

This pull request introduces 3 alerts when merging e46be36 into 6313918 - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null
  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 18, 2022

This pull request introduces 3 alerts when merging 40575dd into 0c5d868 - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null
  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 18, 2022

This pull request introduces 3 alerts when merging acc93ba into 0c5d868 - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null
  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 18, 2022

This pull request introduces 3 alerts when merging 2a70c72 into 0c5d868 - view on LGTM.com

new alerts:

  • 1 for Information exposure through a stack trace
  • 1 for Dereferenced variable may be null
  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 22, 2022

This pull request introduces 1 alert when merging 4af35ae into 1e2e177 - view on LGTM.com

new alerts:

  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 22, 2022

This pull request introduces 1 alert when merging b01d6a6 into 1e2e177 - view on LGTM.com

new alerts:

  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 22, 2022

This pull request introduces 1 alert when merging 2f8e055 into 1e2e177 - view on LGTM.com

new alerts:

  • 1 for Potential input resource leak

@lgtm-com
Copy link

lgtm-com bot commented Feb 22, 2022

This pull request introduces 1 alert when merging 816c050 into cc44dee - view on LGTM.com

new alerts:

  • 1 for Potential input resource leak

@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #3387 (8aa9269) into master (65776bb) will increase coverage by 0.03%.
The diff coverage is 89.00%.

❗ Current head 8aa9269 differs from pull request most recent head ae12b8d. Consider uploading reports for the commit ae12b8d to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3387      +/-   ##
============================================
+ Coverage     82.98%   83.02%   +0.03%     
- Complexity    20746    20859     +113     
============================================
  Files          1396     1400       +4     
  Lines         74540    74875     +335     
  Branches      11163    11176      +13     
============================================
+ Hits          61860    62166     +306     
- Misses         8377     8415      +38     
+ Partials       4303     4294       -9     
Impacted Files Coverage Δ
...-fhir-base/src/main/java/ca/uhn/fhir/i18n/Msg.java 100.00% <ø> (ø)
...src/main/java/ca/uhn/fhir/util/ParametersUtil.java 92.62% <0.00%> (-2.74%) ⬇️
.../uhn/fhir/jpa/cache/ResourceVersionSvcDaoImpl.java 90.74% <ø> (ø)
.../main/java/ca/uhn/fhir/jpa/dao/HistoryBuilder.java 100.00% <ø> (ø)
.../java/ca/uhn/fhir/jpa/dao/LegacySearchBuilder.java 52.61% <ø> (ø)
...hn/fhir/jpa/dao/index/DaoResourceLinkResolver.java 91.74% <ø> (ø)
...ndex/SearchParamWithInlineReferencesExtractor.java 86.70% <ø> (ø)
...java/ca/uhn/fhir/jpa/dao/mdm/MdmLinkExpandSvc.java 10.71% <ø> (ø)
...r/jpa/dao/predicate/PredicateBuilderReference.java 71.06% <ø> (ø)
.../jpa/dao/predicate/PredicateBuilderResourceId.java 71.11% <ø> (ø)
... and 66 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 65776bb...ae12b8d. 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.

Very nice. Using types for chunk input and output is much clearer.

Need to schedule kanban items for:

  • error retry limits
  • avoiding $reindex stalling all other activity

@lgtm-com
Copy link

lgtm-com bot commented Feb 27, 2022

This pull request introduces 2 alerts and fixes 1 when merging 8aa9269 into 65776bb - view on LGTM.com

new alerts:

  • 1 for HTTP response splitting
  • 1 for Potential input resource leak

fixed alerts:

  • 1 for HTTP response splitting

@jamesagnew jamesagnew merged commit b7d1ae2 into master Feb 27, 2022
@jamesagnew jamesagnew deleted the ja_20220208_new_batch_framework branch February 27, 2022 21:04
@lgtm-com
Copy link

lgtm-com bot commented Feb 27, 2022

This pull request introduces 2 alerts and fixes 1 when merging ae12b8d into 65776bb - view on LGTM.com

new alerts:

  • 1 for HTTP response splitting
  • 1 for Potential input resource leak

fixed alerts:

  • 1 for HTTP response splitting

michaelabuckley added a commit that referenced this pull request Feb 28, 2022
* master:
  Fix migration (#3428)
  New batch framework / Bulk Import (#3387)
  Adds a NDJSON-capable Bulk Data Import Provider (#3039)
  small performance optimization (#3426)
  3418 get resource, response doesn't contain total if consent service enabled (#3420)
  Issue 3391 2588 expand valueset regex include not working (#3410)
  Subscription cleanup (#3422)
  Build fix
  Add FK index to avoid timeout when deleting large ValueSet (#3416)
michaelabuckley added a commit that referenced this pull request Mar 1, 2022
…oken-search-index

* commit '2cba62b4e8cac089011124017827af9ab8d96497':
  Enable search narrowing on large ValueSets (#3405)
  Bump core to 5.6.36 (#3433)
  Distinguish multi-coded items during token autocomplete (#3429)
  bump compiler settings.  JDK 11 for prod, JDK 17 for tests (#3435)
  Smile 2775 package install logs ignore resource validation (#3381)
  Add missing fields to canonical subscription compare (#3431)
  Fix migration (#3428)
  New batch framework / Bulk Import (#3387)
  Adds a NDJSON-capable Bulk Data Import Provider (#3039)
  small performance optimization (#3426)
  3418 get resource, response doesn't contain total if consent service enabled (#3420)
  Issue 3391 2588 expand valueset regex include not working (#3410)
  Subscription cleanup (#3422)
  Build fix
  Add FK index to avoid timeout when deleting large ValueSet (#3416)
  Run online index operations non-transactionally on Postgres (#3413)

# Conflicts:
#	hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java
#	hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/ITestDataBuilder.java
@tadgh tadgh restored the ja_20220208_new_batch_framework branch July 23, 2022 19:49
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.

2 participants