Skip to content

Conversation

@jesusbv
Copy link
Collaborator

@jesusbv jesusbv commented May 12, 2025

Description

There is no need to run validations for activations or to instantiate all the activations objects from the DB when destroying a system

Changing :destroy to :delete_all reduce the DB queries considerably, while keeping consistency with regsharing (unaffected)

current queries for deleting a system

INFO: System with login \"SCC_b678091eb6e2496b9dae93376e44b18a\" authenticated, system  token \"i-030b5a7f188ea1821\"
DEBUG:   TRANSACTION (0.1ms)  BEGIN
DEBUG:   Activation Destroy (0.3ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577795
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577796
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577797
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577798
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577799
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577800
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577801
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577802
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   Activation Destroy (0.2ms)  DELETE FROM `activations` WHERE `activations`.`id` = 9577803
DEBUG:   System Exists? (0.2ms) SELECT 1 AS one FROM `systems` WHERE `systems`.`system_token` = 'i-030b5a7f188ea1821' AND `systems`.`id` != 891691 AND `systems`.`login` = 'SCC_b678091eb6e2496b9dae93376e44b18a' AND `systems`.`password` = 'b3ea7beda78f46b3' LIMIT 1
DEBUG:   SystemUptime Load (0.2ms) SELECT `system_uptimes`.* FROM `system_uptimes` WHERE `system_uptimes`.`system_id` = 891691
DEBUG:   System Destroy (0.2ms)  DELETE FROM `systems` WHERE `systems`.`id` = 891691
DEBUG:   TRANSACTION (1.0ms)  COMMIT
INFO: Completed 204 No Content in 36ms (ActiveRecord: 7.2ms | Allocations: 12523)

if we apply this change

INFO: System with login \"SCC_93eea65b0b324dbeb4bb2928b9a20144\" authenticated, system  token \"i-030b5a7f188ea1821\"
DEBUG:   TRANSACTION (0.1ms)BEGIN
DEBUG:   Activation Destroy (0.4ms) DELETE FROM `activations` WHERE `activations`.`system_id` = 891799
DEBUG:   SystemUptime Destroy (0.1ms) DELETE FROM `system_uptimes` WHERE `system_uptimes`.`system_id` = 891800
DEBUG:   System Destroy (0.2ms) DELETE FROM `systems` WHERE `systems`.`id` = 891799
DEBUG:   TRANSACTION (1.0ms) COMMIT
INFO: Completed 204 No Content in 14ms (ActiveRecord: 4.2ms | Allocations: 5739)

Implications or side effects:

  • Registration sharing:

    Non issue,

    • CASE A:
      if both update infrastructure servers are on the same version, meaning having this change of queries
      when deleting the system will trigger a registration sharing on the sibling, the sibling will destroy the sibling and produce the same 2 queries

      removing activation does not change behaviour
      when an activation is deleted in one update server, the destruction of that activation is still propagated on regsharing

    • CASE B:
      if sibling 1 has the feature and sibling 2 has the multiple queries for activations (destroy on dependency),
      when deleting the system on the server with the new feature will trigger the destruction of the system on the server without the feature and nothing will have changed for
      that operation on the sibling without the feature

      same the other way around, if server without the feature removes a system will trigger the destruction of the system on regsharing for the sibling with the new feature,
      producing only 2 queries (if the system had any activation, that is)

      removing activation does not change behaviour

  • **Callbacks (includes regsharing call)*:

    • CASE A:
      trigger DeregisteredSystem.find_or_create_by system.scc_system_id does not change behaviour as it depends on system destroy call, which is still in place

    • CASE B:
      triggering DeregisteredSystem.find_or_create_by system.scc_system_id does not change behaviour as it depends on system destroy call, which is still in place

  • Related Issue / Ticket / Trello card: part of jsc#PCT-488

How to test

Running registercloudguest --clean on the client should change nothing on the behaviour and operations in the update infrastructure server:

  • the system should get removed
  • its activations should get removed
  • registration sharing should trigger the removal of the system to its siblings
  • independently of the dependant setting of the relation on the DB the activations "
    of the systems should get removed in the siblings

Change Type

Please select the correct option.

  • Bug Fix (a non-breaking change which fixes an issue)
  • New Feature (a non-breaking change which adds new functionality)
  • Documentation Update (a change which only updates documentation)

Checklist

Please check off each item if the requirement is met.

  • I have reviewed my own code and believe that it's ready for an external review.
  • I have provided comments for any hard-to-understand code.
  • I have documented the MANUAL.md file with any changes to the user experience.
  • If my changes are non-trivial, I have added a changelog entry to notify users at package/obs/rmt-server.changes.

Review

Please check out our review guidelines
and get in touch with the author to get a shared understanding of the change.

@jesusbv jesusbv requested review from digitaltom and rjschwei May 12, 2025 09:35
@jesusbv jesusbv self-assigned this May 12, 2025
There is no need to run validations for activations or to instantiate
all the activations objects from the DB when destroying a system

Changing destroy to delete_all reduce the DB queries considerably,
while keeping consistency with regsharing (unaffected)
@jesusbv jesusbv force-pushed the fewer-queries-clean-system branch from ff0d8e1 to 7e08f2f Compare May 12, 2025 09:51
@jesusbv jesusbv added the 2.23 label Jun 10, 2025
@jesusbv jesusbv merged commit befe5eb into master Jun 12, 2025
3 of 4 checks passed
@jesusbv jesusbv deleted the fewer-queries-clean-system branch June 12, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants