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

Use the new is_deleted and last_updated columns from xx_LOGICAL_RESOURCES to eliminate joins #1385

Closed
punktilious opened this issue Jul 30, 2020 · 4 comments
Assignees
Labels
P2 Priority 2 - Should Have performance performance reference-search search showcase Used to Identify End-of-Sprint Demos

Comments

@punktilious
Copy link
Collaborator

punktilious commented Jul 30, 2020

Is your feature request related to a problem? Please describe.
Searches always need to join to the _RESOURCES table to filter on the value for IS_DELETED. This flag represents the current state of the logical resource and so should be defined at the LOGICAL_RESOURCE level, not the RESOURCE (which represents a version of LOGICAL_RESOURCE.

Describe the solution you'd like
Investigate if it should be added to the global table rather than the resource-specific table.
Add the column to the logical resources table
Migrate data
Remove column from the resources tables
reorg
Update the search query generation as required, removing joins to the resources table unless they are required. The only reason to join with resources should be when the lastUpdated value is needed, and the final join to fetch the data blob.

Team discussion - added a comment that we have to consider VERSION specific status which is also on the RESOURCES table.

Describe alternatives you've considered
Status quo, and accept more expensive joins.

Additional context
N/A

Implementation Note
Trace specifications. To see search query timings:

com.ibm.fhir.persistence.jdbc.dao.impl.FHIRDbDAOImpl.level=FINE

To see search query SQL statements:

com.ibm.fhir.database.utils.query.QueryUtil.level=FINE
@prb112 prb112 changed the title Moving is_deleted flag to logical_resources will eliminate many joins to resources tables Adding is_deleted flag to logical_resources will eliminate many joins to resources tables Oct 5, 2020
@lmsurpre
Copy link
Member

lmsurpre commented Nov 13, 2020

Team discussion - added a comment that we have to consider VERSION specific status which is also on the RESOURCES table.

I think what that means is that we would need IS_DELETED on both the RESOURCES and LOGICAL_RESOURCES tables.
The one on RESOURCES will continue to mean that this particular version of the resource is deleted.
The one on LOGICAL_RESOURCES will indicate whether the "latest" version is deleted

@lmsurpre
Copy link
Member

lmsurpre commented Mar 2, 2021

This flag was added under #1958, but work remains to actually use it.

@lmsurpre
Copy link
Member

lmsurpre commented Mar 4, 2021

Now that we have #2014 I'm not sure if we still need this one. If so, how do they relate?

@lmsurpre lmsurpre changed the title Adding is_deleted flag to logical_resources will eliminate many joins to resources tables Use the new is_deleted and last_updated columns from xx_LOGICAL_RESOURCES to eliminate joins Mar 4, 2021
@lmsurpre lmsurpre added the P2 Priority 2 - Should Have label Mar 4, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-04 milestone Mar 15, 2021
@punktilious punktilious self-assigned this Mar 16, 2021
lmsurpre added a commit that referenced this issue Apr 1, 2021
Its not super clean, but I wanted to see if it was possible to get some
isolated benefits from #1385 without needing to adopt the new query
builder.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@d0roppe
Copy link
Collaborator

d0roppe commented May 11, 2021

Ran the performance bucket against this code and all searches were processed. No odd behavior. Closing issue as the new code seems to be working.

@d0roppe d0roppe closed this as completed May 11, 2021
@lmsurpre lmsurpre added the showcase Used to Identify End-of-Sprint Demos label May 11, 2021
@punktilious punktilious removed the schema-change a schema change label May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Priority 2 - Should Have performance performance reference-search search showcase Used to Identify End-of-Sprint Demos
Projects
None yet
Development

No branches or pull requests

4 participants