-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Remove host update code which can be non performant #14233
Conversation
Manually testing to booster confidence.
before deletion {
"id": 104,
"type": "job_event",
"url": "[/api/v2/job_events/104/](https://localhost:8043/api/v2/job_events/104/)",
"related": {
"job": "[/api/v2/jobs/60/](https://localhost:8043/api/v2/jobs/60/)",
"children": "[/api/v2/job_events/104/children/](https://localhost:8043/api/v2/job_events/104/children/)",
"host": "[/api/v2/hosts/1/](https://localhost:8043/api/v2/hosts/1/)"
},
"summary_fields": {
"host": {
"id": 1,
"name": "localhost",
"description": ""
},
"job": {
"id": 60,
"name": "Demo Job Template",
"description": "",
"status": "successful",
"failed": false,
"elapsed": 5.219,
"type": "job",
"job_template_id": 7,
"job_template_name": "Demo Job Template"
},
"role": {}
},
"created": "2023-07-12T20:23:02.413242Z",
"modified": "2023-07-12T20:23:02.657922Z",
"job": 60,
"event": "runner_on_start",
"counter": 4,
"event_display": "Host Started",
"event_data": {
"playbook": "hello_world.yml",
"playbook_uuid": "50a201ec-a817-4b94-a71a-9388c68f1b1c",
"play": "Hello World Sample",
"play_uuid": "92042dfb-5402-072f-efd8-000000000002",
"play_pattern": "all",
"task": "Gathering Facts",
"task_uuid": "92042dfb-5402-072f-efd8-000000000008",
"task_action": "gather_facts",
"resolved_action": "ansible.builtin.gather_facts",
"task_args": "",
"task_path": "/runner/project/hello_world.yml:1",
"host": "localhost",
"uuid": "5f39f0d0-8400-4c44-b085-b76949402a1d",
"guid": "84b0ee1ffa5f4c3f81310913953da1be"
},
"event_level": 3,
"failed": false,
"changed": false,
"uuid": "5f39f0d0-8400-4c44-b085-b76949402a1d",
"parent_uuid": "92042dfb-5402-072f-efd8-000000000008",
"host": 1,
"host_name": "localhost",
"playbook": "hello_world.yml",
"play": "Hello World Sample",
"task": "Gathering Facts",
"role": "",
"stdout": "",
"start_line": 4,
"end_line": 4,
"verbosity": 0
}, after {
"id": 104,
"type": "job_event",
"url": "[/api/v2/job_events/104/](https://localhost:8043/api/v2/job_events/104/)",
"related": {
"job": "[/api/v2/jobs/60/](https://localhost:8043/api/v2/jobs/60/)",
"children": "[/api/v2/job_events/104/children/](https://localhost:8043/api/v2/job_events/104/children/)",
"host": "[/api/v2/hosts/1/](https://localhost:8043/api/v2/hosts/1/)"
},
"summary_fields": {
"job": {
"id": 60,
"name": "Demo Job Template",
"description": "",
"status": "successful",
"failed": false,
"elapsed": 5.219,
"type": "job",
"job_template_id": 7,
"job_template_name": "Demo Job Template"
},
"role": {}
},
"created": "2023-07-12T20:23:02.413242Z",
"modified": "2023-07-12T20:23:02.657922Z",
"job": 60,
"event": "runner_on_start",
"counter": 4,
"event_display": "Host Started",
"event_data": {
"playbook": "hello_world.yml",
"playbook_uuid": "50a201ec-a817-4b94-a71a-9388c68f1b1c",
"play": "Hello World Sample",
"play_uuid": "92042dfb-5402-072f-efd8-000000000002",
"play_pattern": "all",
"task": "Gathering Facts",
"task_uuid": "92042dfb-5402-072f-efd8-000000000008",
"task_action": "gather_facts",
"resolved_action": "ansible.builtin.gather_facts",
"task_args": "",
"task_path": "/runner/project/hello_world.yml:1",
"host": "localhost",
"uuid": "5f39f0d0-8400-4c44-b085-b76949402a1d",
"guid": "84b0ee1ffa5f4c3f81310913953da1be"
},
"event_level": 3,
"failed": false,
"changed": false,
"uuid": "5f39f0d0-8400-4c44-b085-b76949402a1d",
"parent_uuid": "92042dfb-5402-072f-efd8-000000000008",
"host": 1,
"host_name": "localhost",
"playbook": "hello_world.yml",
"play": "Hello World Sample",
"task": "Gathering Facts",
"role": "",
"stdout": "",
"start_line": 4,
"end_line": 4,
"verbosity": 0
}, This legit gives a 404 when clicking on the host link. I don't think I care. |
Also checked running the cleanup jobs management job effectively removes the job with a deleted inventory. It does. Also confirmed I am left with 0 job events in the system. I am. Done with manual testing, I'm fine with this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see anywhere that we set cascade on this field which is probably why the host sticks in Alans testing but yields a 404. But I am also fine with that.
I wanted to formalize the shape of the query we're looking at removing here, because this is really helpful if someone is looking at SQL statements and trying to map them to the source. I had to dig into the Django source code because I can't just do So to test this query, I do: from django.db.models import sql
j = Job.objects.order_by('-created').first()
h = j.inventory.hosts.first()
h.job_events_as_primary_host.all()
qs = h.job_events_as_primary_host.all()
qs._not_support_combined_queries("update")
qs._for_write = True
query = qs.query.chain(sql.UpdateQuery)
query.add_update_values({'host': None})
print(str(query)) Doing this, I get: UPDATE "main_jobevent" SET "host_id" = NULL WHERE "main_jobevent"."host_id" = 1 So this is a query that should no longer be seen after this merge. It should probably never be done. |
SUMMARY
#14231
Inventory deletion causes unnecessary queries to the database to update fields which isn't necessary. This removes that update before deletion.
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION