iTop icon indicating copy to clipboard operation
iTop copied to clipboard

N°4342 - Generic bulk deletion function with memory limit handling

Open accognet opened this issue 3 years ago • 2 comments

Change function PurgeData in order to delete data by lot

accognet avatar Aug 08 '22 12:08 accognet

I feel like we should rename the conf parameter to be more explicit about what it does and its usage. It's not a generic buffer size but a "chunk size" or "batch size" for purging data.

Also, why didn't added the conf parameter like the others in the big array instead of a constant?

Molkobain avatar Sep 01 '22 06:09 Molkobain

Discussed during technical review today.

Romain and Anne-Catherine will do a pre-review together. In particular we need to be sure :

  • where is this \MetaModel::PurgeData mehod used in iTop ?
  • why do we have a memory limit error ? Does the GetColumnAsArray() will load the whole objects besides we only asked for 2 fields (id and finalclass) ?

About the config parameter :

  • using an attribute + a getter is the "old way". @accognet please move the declaration to the \Config::$m_aSettings instead
  • the parameter shouldn't be visible by default (show_in_conf_sample array key)
  • the param name could be purge_data.chunk_size ?
  • default value could be 1000

piRGoif avatar Sep 06 '22 16:09 piRGoif

Hi guys, I am one of those who asked for a generic bulk deletion solution a while back, in Combodo's ticket R-032835 (which is linked to ER N°4342). But discovering and reading the content of this PR, while I'm happy something is being done on this front, I realize we might not mean the same :

As far as I understand the code, PurgeData will do only a brutal SQL purge of the object matching the provided filter. But it will NOT guarantee data integrity because it will not do a proper update or delete of related data (update N:N link tables and related objects' history, purge deleted objects' related CMDBChangeOp, block deletions not respecting mandatory link with manual deletion, etc.). As its comments state, "it bypasses standard objects handlers". So it seems this current work would be useless for the use case where the data to be purged has datamodel dependencies (so any instance of class inheriting from cmdbAbstractObject).

What I meant when asking for a generic bulk deletion what to be able to mass delete any instance of classes inheriting cmdbAbstractObject, while respecting all iTop data handeling standards. So it would need to perform all iTop's standard data management to respect data integrity and proper historysation (like any deletionPlan would do). I saw that last version 1.3 of PersonnalDataAnonymizer extension includes some sort of such a batch mass data processing but at the moment I could not find any documentation on how it could be used outside of personal data anonymation for mass delete.

If that's ok with you, I would be interested to discuss this matter with you in any way you would see fit.

favincen avatar Aug 29 '23 13:08 favincen

Hi @favincen, some of the developers are available on the iTop community Slack team.

Hipska avatar Aug 29 '23 13:08 Hipska

Remaining tasks:

  • Add a unit test on the method
  • Apply suggestions

Molkobain avatar Feb 26 '24 09:02 Molkobain

Great, you can "squash & merge" !

Molkobain avatar Feb 27 '24 19:02 Molkobain