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

Avoid redundant reads during update interaction #2283

Closed
lmsurpre opened this issue Apr 23, 2021 · 3 comments
Closed

Avoid redundant reads during update interaction #2283

lmsurpre opened this issue Apr 23, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request P2 Priority 2 - Should Have

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Apr 23, 2021

Is your feature request related to a problem? Please describe.
Doing a single read before the update makes sense, because we need to pull the current version out of the db to set the new version.
However, presently, we do this read multiple times.

Firstly, around line 432 in my local version, we do this:

        ior.setPrevResource(doRead(type, id, (patch != null), true, newResource, null, false));

Then, a little lower (around line 460 in FHIRRestHelper), we do it again:

                try {
                    doRead(type, id, (patch != null), false, newResource, null, false);
                } catch (FHIRPersistenceResourceDeletedException e) {
                    isDeleted = true;
                }

The second read is just to see if its deleted or not.
Is there a better way?

Describe the solution you'd like
Determine whether the resource is deleted or not the first time we read it. Probably needs design.

Describe alternatives you've considered
Push more of this into the persistence layer

Acceptance Criteria

Additional context

@lmsurpre lmsurpre changed the title Update interaction reads the resource twice before executing Avoid redundant reads during update interaction Apr 23, 2021
@prb112 prb112 added the enhancement New feature or request label Apr 26, 2021
@lmsurpre lmsurpre added the P2 Priority 2 - Should Have label May 3, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-07 milestone May 24, 2021
@punktilious punktilious self-assigned this May 24, 2021
@punktilious
Copy link
Collaborator

punktilious commented May 25, 2021

There is a better way...

    SingleResourceResult<? extends Resource> doRead(String type, String id, boolean throwExcOnNull, boolean includeDeleted,
            Resource contextResource, MultivaluedMap<String, String> queryParameters) throws Exception;

The doRead functions now return a SingleResourceResult which has been augmented with the deleted status flag. The read is now only performed once...

(when the change associated with this issue is merged)

@lmsurpre
Copy link
Member Author

lmsurpre commented Jun 7, 2021

I found another case where we are performing a redundant read: #2478

@lmsurpre
Copy link
Member Author

lmsurpre commented Jun 7, 2021

I ran the server in debug mode and confirmed we only perform a single update now (in the case of a "true" update).

@lmsurpre lmsurpre closed this as completed Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 Priority 2 - Should Have
Projects
None yet
Development

No branches or pull requests

3 participants