Skip to content

add dependency checks for location deletion for better error messages#197

Open
adamkorynta wants to merge 1 commit into
mainfrom
bugfix/location_delete
Open

add dependency checks for location deletion for better error messages#197
adamkorynta wants to merge 1 commit into
mainfrom
bugfix/location_delete

Conversation

@adamkorynta

@adamkorynta adamkorynta commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

add in savepoint and rollback on delete and ensure cache removal happens last

Fixes #182

add in savepoint and rollback on delete and ensure cache removal happens last
@adamkorynta adamkorynta requested a review from perrymanmd July 8, 2026 18:58
@adamkorynta

Copy link
Copy Markdown
Contributor Author

@perrymanmd - I could not reproduce the issue with schema 25.7.1 or the 26.02.17-RC02. I'm wondering if the updates done for the geometry might have inadvertently solved the issue. I tested deleting both base locations and sublocations. I think adding some error handling and a save point might help diagnose the issues if they persist. I did a little reorganizing of the delete_location method. Can you take a look?

@MikeNeilson

Copy link
Copy Markdown
Contributor

I'm wondering if the updates done for the geometry might have inadvertently solved the issue.

That would not surprise me.

@adamkorynta adamkorynta marked this pull request as ready for review July 9, 2026 22:20
@adamkorynta adamkorynta requested review from a team, rma-rripken and tlelv and removed request for a team, rma-rripken and tlelv July 9, 2026 22:21
end if;

if l_delete_action in (cwms_util.delete_key, cwms_util.delete_loc) then
l_dependency_info := get_delete_dependency_info;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this seems like something where we shouldn't do it until the error is raised.

Though I'll grant anyone doing deletes in a tight loop regularly is not doing something remotely sane.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the intention here was to ward off the reported deadlock and figured the performance hit wouldn't make anyone grumble too much

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.

Deadlock reported on Location delete at NWDM via T7 CDA

3 participants