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

Rtirabassi uid map oom #2447

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rtirabassi
Copy link

Dear Gunter,
I made some changes (starting from 5.21.0) to solve the following problems.
Since I'm novice to dcm4chee-arc-light (and to DICOM world too) please forgive me if my terms are uncorrect.

  1. Suppose to have a study with a large amount (say > 1000) of instances (and relative locations). Than suppose to move this study from a patient to another. A single UIDMap is created to collect all ID changes.
    Now let's export this study (e.g. using movescu) from this PACS to another.
    The LocationQuery collects many columns and the UIDMap column too (> 1000 times). This cause the query execution to run OutOfMemory. So I removed the UIDMap column from the query and applied a simple cache to reuse each UIDMap (normally 1, if study in not copied more than once) in every location that is created from the query result.

  2. Let's move a study from a patient to another with a 'Incorrect MWL Entry' reject type.
    DICOM File is now referenced from more than one Instance (so, more than one location): the one corresponding to original patient (but in rejected state) and the new one. Both of the locations are marked with a multiReference ID.
    Now move that study again, back to the same patient or to another one, with same reject type.
    The DICOM File is now referenced from three locations: two of them are in rejected state and just one is actually "valid" (as supposed).
    Here's the scenario, now let's see what the problem is.
    Deleting permanently the rejected instances cause the valid DICOM File to be removed.
    This is due to:
    a.1) When location is copied the second time, the multiReference field is not populated since location has not been loaded from the DB but built during LocationQuery execution. So the StoreServiceEJB.copyLocation() first checks for multiReference presence, than loads the location from DB... but doesn' t check it again. This means that newLocationMultiReference() is called on pevious location (and value is then assigned to the new one) substituting the existing value and breaking the multiReference chain. This way, the second and third location will share the same multiReference ID but the first one will preserve a different one and can be "Marked for Deletion" (while other ones are simply removed until the chain has more than 1 ring). The removed file will also affect remaining instances/locations and the corresponding DICOM File is lost forever.
    Checking again multiReference value after location reload before assigning a new ID keeps the multiReference chain safe.
    a.2) Since the multiReference ID is a simple Integer value, I also added it to the LocationQuery columns in order to have that value pre-loaded and avoid reloading the location from DB during copy activity. I think this could fasten up copy process, but 'a.1' extra check is safer anyway.
    b) When a location (and it's DICOM file) is candidate for deletion, according to what said at point 'a.1', corresponding DICOM file is removed with no other checks. This leads to a loss of file related to valid locations in this example, expecially before I find and solved point 'a.1'. In order not to risk to loose a file for any reason, I added a 'hasInUseFile()' check to avoid deletion of a "still in use" file.

  3. This is a feature request: when study is moved as in previous examples, metadata only are still available in the old study. I still can walk the path up to the instance and see metadata, but I can also ask to "View DICOM File" that does not contain PIxelData anymore.
    The result is a new blank tab in the browser telling something went wrong.
    I suppose the corresponding button could be disalbed or hide.

Thanks in advance.

add: set UIDMap cache into LocationQuery and same reference usage in order to save RAM.
… when asked, but file is not if still in use.
fix: When previous location is reloaded to check multiReference presence, than the multiReference wasn't tested and a new multiReference ID assigned, this way multi reference chain is broken.
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.

None yet

3 participants