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

deleting reaches: disabling symbology triggers leaves dead data in the database (reach_points etc) #485

Open
urskaufmann opened this issue Oct 31, 2024 · 10 comments · May be fixed by #491
Open
Labels
bug Something isn't working critical performance

Comments

@urskaufmann
Copy link
Contributor

Describe the bug

  • open a new database prerelease 2024.03

  • import xtf organisation and data
    Info:   1611 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Abwasserbauwerk_Text
    Info:   2273 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Abwasserknoten
    Info:   1535 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Deckel
    Info:   90 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Einleitstelle
    Info:   126 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Einstiegshilfe
    Info:   2163 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Haltung
    Info:   1519 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Haltung_Text
    Info:   4326 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Haltungspunkt
    Info:   2073 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Kanal
    Info:   1809 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Normschacht
    Info:   9 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Rohrprofil
    Info:   25 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Spezialbauwerk
    Info:   2 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Streichwehr
    Info:   2 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Trockenwetterfallrohr
    Info:   12 objects in CLASS DSS_2020_1_LV95.Siedlungsentwaesserung.Unterhalt

  • Watch this data in TWW

  • Important: disable symbology triggers (otherways it lasts hours)

  • delete all records in vw_tww_wastewater_structure (1924)

  • delete all records in vw_tww_reach (2163)

  • delete all records in vw_wastewater_node (371)

-> no more visible records in TWW

  • controle with pgAdmin, how many records are still in tww_od-tables:
  • wastewater_structure: 2073
  • channel: 2073
  • reach: 0
  • reach_point: 4326
  • wastewater_node 0
  • wastewater_networkelement: 2163
  • structure_part, cover: 0

Conclusion: deleting in vw_tww_reach leaves dead data in classes reach_point, channel, networkelement and wastewater_structure

Expected behavior
all related data is also deleted.
Is this bug because of the disabled trigger?

Screenshots / data
image

Desktop (please complete the following information):

  • TWW version 2024.03 dev
  • QGIS Version 3.34.10
  • OS Win 10
@urskaufmann
Copy link
Contributor Author

If I delete just same reaches without disabling symbology triggers before, the reach_point are also deleted.

What can we do, that we do not get dead data because of deleting records while the triggers are disabled?
Deleting reach_points when deleting reaches is a must! No exception.
The same in superclass-subclass-relation, both must be deleted, when one record is deleted.
It's a bit more difficult with the reaches and channels...

@urskaufmann urskaufmann changed the title deleting reaches: not all connected data is deleted deleting reaches: disabling symbology triggers leaves dead data in the database (reach_points etc) Nov 4, 2024
@ponceta ponceta added bug Something isn't working critical labels Nov 5, 2024
@ponceta
Copy link
Member

ponceta commented Nov 5, 2024

@cymed I think we have to clarify application triggers (which dispatch the data to the right places) with symbology triggers.

Main difference being :
symobology triggers can be run anytime as they are just updating fields and stuff for symbology purposes

application triggers can't be simply deactivated or only by a sys_admin knowing which operation can be operated when these are deactivated.

@ponceta
Copy link
Member

ponceta commented Nov 5, 2024

@urskaufmann I think the performance issue is also a high priority point to assess, we made some performance assessment for QWAT at the time we implemented the TRIGGER / VIEW approach, there's no reason the edition performance should be so low as we have to disable application functionalities to make simple edition process.

@cymed
Copy link
Contributor

cymed commented Nov 5, 2024

@cymed I think we have to clarify application triggers (which dispatch the data to the right places) with symbology triggers.
The behaviour has to do with symbology triggers indirectly:

We do not have an instead of trigger on tww_app.vw_tww_reach but a rule vw_tww_reach_delete that deletes the reach only. On delete of the reach we have a trigger tww_app.symbology_on_reach_delete that was disabled in the old tww_app.disable_symbology_trigger() and thus was disabled in my alteration. If this trigger deletes the networkelement (no effect on symbology), the reach points (calling a symbology trigger) and the channel if it is no longer referenced by the reach (firing another symbology trigger)

Main difference being : symobology triggers can be run anytime as they are just updating fields and stuff for symbology purposes

application triggers can't be simply deactivated or only by a sys_admin knowing which operation can be operated when these are deactivated.

@ponceta
Copy link
Member

ponceta commented Nov 11, 2024

We have to clearly define what is symbology and what is not.

In QGEP the term symbology was already used but all of them where dropped and recreated before and after any upgrade task (and they were dropped, not only disabled).

https://github.com/QGEP/datamodel/blob/124e25279988a97b347989f83ca291cf5297d5cc/06_symbology_functions.sql#L790-L811

Which is now :

We will then have two trigger functions types:
disable/enable_symbology_triggers
disable/enable_modification_triggers

Since both are on an application triggers.
For symbology loops in system triggers, I would add a check/CASE to loop only if symbology triggers are activated.

All of the triggers and rules affecting the datamodel should be documented somewhere to avoid any surprise behaviour.

@ponceta ponceta linked a pull request Nov 11, 2024 that will close this issue
14 tasks
@ponceta
Copy link
Member

ponceta commented Nov 13, 2024

@3nids @domi4484
I would need advice from your side :

  1. Why having a rule instead of a delete trigger only for the vw_tww_reach view?
  2. Should we have more triggers to ease the separation between the modification and symbology tasks?
  3. Is the functions / trigger separation a good idea?

Could comment and evaluate what is proposed on #491 and what should be adapted to avoid such issues in the future?

@cymed
Copy link
Contributor

cymed commented Nov 13, 2024

It needs to be a rule, because we only want to delete the channel if there are no other reaches with a fk on that channel.

@domi4484
Copy link
Contributor

If we disable triggers or functions it is normal that something will stop working. This is also why by default this options are disabled and available only for admins. Btw. I would propose to put those actions a bit more hidden into the Settings -> Advanced menu
image

Another thing to take care, is that if you disable the triggers, those are disabled for everyone working on the same database and not only for you.

Probably I am missing some background, but for me the main question is why do we need to disable the triggers? Because of performance?

  • Do we know specifically which triggers make things slow? Only the symbology or the modification as well?
  • Is the slowness at database level or in QGIS?
  • Is it slow only for bulk actions (e.g. deleting of multiple objects at once) or always?
  • Did we already tried to improve that?

@ponceta
Copy link
Member

ponceta commented Nov 18, 2024

The general idea was to disable only symbology triggers (the ones doing an update that can be done anytime on the whole database)

Theses triggers are often taking loops and stuff that can lead to performance issues when dealing with several features.

- Do we know specifically which triggers make things slow? Only the symbology or the modification as well?
  • No we don't know it yet / There is no performance / scallability on QGEP / TEKSI wastewater but they exist on QWAT

    • Is the slowness at database level or in QGIS?
  • Database level, any general attribute upgrade is taking too much time

    • Is it slow only for bulk actions (e.g. deleting of multiple objects at once) or always?
  • Yes, mainly for bulk actions, deactivating symbology triggers was a proposed workaround

    • Did we already tried to improve that?
  • Not yet but we will have to do it in the future, the future could be now.

@cymed
Copy link
Contributor

cymed commented Nov 18, 2024

Potential improvements include

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants