Skip to content

fix: cinder potential duplicate volume metrics#110

Open
Tadas Sutkaitis (fitbeard) wants to merge 1 commit into
mainfrom
fix/cinder_duplicated_metrics
Open

fix: cinder potential duplicate volume metrics#110
Tadas Sutkaitis (fitbeard) wants to merge 1 commit into
mainfrom
fix/cinder_duplicated_metrics

Conversation

@fitbeard
Copy link
Copy Markdown
Contributor

Fix: #95

Add: Initial AGENTS.md

Signed-off-by: Tadas Sutkaitis <tadas.sutkaitis@vexxhost.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8901f2bf23

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread sql/cinder/queries.sql
volumes v USE INDEX (volumes_service_uuid_idx)
LEFT JOIN volume_types vt ON v.volume_type_id = vt.id
LEFT JOIN volume_attachment va ON v.id = va.volume_id AND va.deleted = 0
LEFT JOIN volume_attachment va ON v.id = va.volume_id AND va.deleted = 0 AND va.attach_status = 'attached'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not drop attachments with NULL attach_status

In deployments or fixtures where volume_attachment.attach_status is still NULL for a non-deleted attachment, this join no longer matches the row and the exporter emits server_id="" for an in-use volume even though instance_uuid is present. The column is nullable/defaults to NULL in the repo schema, and the end-to-end Cinder seed in cmd/openstack-database-exporter/main_test.go currently inserts an attachment without attach_status, so this change silently loses the server label for that valid row shape rather than only filtering detached attachments.

Useful? React with 👍 / 👎.

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.

After creating and attaching the volume, the real DB shows attach_status='attached', not NULL.

Key results:

attach_status  deleted  rows_total  rows_with_instance_uuid
attached       0        1           1
detached       1        149         143

Join comparison:

old join:              rows=1 volumes=1 rows_with_instance=1
strict attached join:  rows=1 volumes=1 rows_with_instance=1
NULL-inclusive join:   rows=1 volumes=1 rows_with_instance=1

Latest active attachment:

volume status: in-use
volume attach_status: attached
attachment_status: attached
instance_uuid: present
deleted: 0

So on current real Cinder behavior, strict va.attach_status = 'attached' works and does not drop the server label.

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.

Potential duplicate volume metrics

1 participant