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

Reduce storage required for indexing - stop writing sp_name, res_type, and sp_updated to hfj_spidx_* tables #5941

Merged

Conversation

volodymyr-korzh
Copy link
Collaborator

@volodymyr-korzh volodymyr-korzh commented May 15, 2024

Migration:

  • migrated all HFJ_SPIDX tables to allow SP_NAME and RES_TYPE columns to be nullable.
  • Migration runs with failureAllowed(), as both SP_NAME and RES_TYPE could be included in custom indexes, and SQL Server won't let us change nullability on columns with indexes pointing to them.

Optimization changes:

  • added IndexStorageOptimizationListener - it is applied only to HFJ_SPIDX tables - nulling SP_NAME, RES_TYPE SP_UPDATED if this feature is enabled.
  • IndexStorageOptimizationListener uses JpaSearchParamCache to recover SP_NAME, RES_TYPE from hash_identity after loading from DB.
  • HASH_IDENTITY field was added to BaseResourceIndexedSearchParam entity and removed from all inheritor entities.
  • Equals and hashCode methods for all ResourceIndexedSearchParam now using HASH_IDENTITY instead of sp_name and res_type. This is required to make ResourceIndexedSearchParam objects with and without optimization to be equal - to not cause unnecessary ResourceIndexedSearchParams updates. (as we are comparing db version of entities with in-memory built Search params)
  • Updated DaoSearchParamSynchronizer logic to check whether it is needed to update existing search parameters after isIndexStorageOptimized change
  • Exception is thrown during the startup if isIncludePartitionInSearchHashes and isIndexStorageOptimized are enabled on server. (isIncludePartitionInSearchHashes is not supported if isIndexStorageOptimized is set to true)
  • InMemoryResourceMatcher now uses hashIdentity to filter SearchParams instead of sp_name. (only if optimization is enabled)
  • BaseSearchParamPredicateBuilder now uses hashIdentity instead of SP_NAME, RES_TYPE to build a query. This way new optimization could work in pair with Enabled IndexMissingFields setting. (only if optimization is enabled)
  • Added new tests and documentation.

…_type and sp_updated columns of HFJ_SPIDX tables nullable
Copy link

github-actions bot commented May 15, 2024

Formatting check succeeded!

Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.51%. Comparing base (497b9f2) to head (263d02f).
Report is 106 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5941      +/-   ##
============================================
+ Coverage     83.39%   83.51%   +0.11%     
- Complexity    26927    27324     +397     
============================================
  Files          1681     1701      +20     
  Lines        103965   105751    +1786     
  Branches      13189    13351     +162     
============================================
+ Hits          86702    88315    +1613     
- Misses        11613    11729     +116     
- Partials       5650     5707      +57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@volodymyr-korzh volodymyr-korzh marked this pull request as ready for review May 30, 2024 17:41
@volodymyr-korzh volodymyr-korzh requested a review from a team as a code owner May 30, 2024 17:41
Copy link
Collaborator

@tadgh tadgh left a comment

Choose a reason for hiding this comment

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

Approved pending various comments.

…-required-for-indexing-tables

# Conflicts:
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamCoordsTest.java
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamDateTest.java
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamQuantityNormalizedTest.java
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamQuantityTest.java
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamStringTest.java
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamTokenTest.java
#	hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedSearchParamUriTest.java
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.

Good work.

Reading your work exposed some missing tests around the entity equals/hashCode work.
Made some minor suggestions.
Remember to restore settings changed Spring contexts after the test.

@volodymyr-korzh volodymyr-korzh merged commit 0397b9d into master Jun 20, 2024
66 checks passed
@volodymyr-korzh volodymyr-korzh deleted the 5937-reduce-storage-required-for-indexing-tables branch June 20, 2024 20:10
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.

Reduce storage required for indexing - stop writing sp_name, res_type, and sp_updated to hfj_spidx_* tables
4 participants