WIP: [SAP] Add volume history tracking#319
Open
hemna wants to merge 9 commits intostable/2025.1-m3from
Open
Conversation
Add a new VolumeHistory model to track changes to volumes over time. Each record captures a JSON delta of changed fields (old/new values) along with contextual metadata (user_id, project_id, request_id). Change-Id: Ib676eb81507847393183fe05f9870266b79d5b4b
Creates the volume_history table with columns for tracking volume changes over time including volume_id, project_id, user_id, request_id, action type, and a JSON changes column for the delta. Change-Id: Ifa0e979f0c6746a13f5a64c6dd15565f949ebef3
- Add import json for serializing history changes - Add VolumeHistory to VOLUME_DEPENDENT_MODELS for cascade soft-delete - Add _record_volume_history() helper function - Instrument volume_create() to record initial values - Instrument volume_update() to record delta of changed fields - Instrument volume_destroy() to record status->deleted transition - Instrument volume_attached() to record status/attach_status changes - Instrument volume_detached() to record status/attach_status changes - Add volume_history_get_all_by_volume() query function - Add pass-through in db/api.py Change-Id: I60fcd9c887860bd78b1124b2c79a4e2de891c0ec
Verify the migration creates the volume_history table with all expected columns and the volume_id index. Change-Id: I13ca352590632d836b870377c46e64a4855507b3
Add DBAPIVolumeHistoryTestCase with tests for: - Volume create records history - Volume update records history with changed fields - Volume update with no changes doesn't record history - Volume destroy records history - History records are ordered by created_at - Request ID is captured in history - Getting history for non-existent volume returns empty list Change-Id: I8d2e773eeafdaee483667f9dcb029b2eddef3b03
Change-Id: Ifdec8eccf9e7ac4a1f438d166096a9d5f1ef37bd
Add a config option to disable volume history tracking for high-throughput environments where the additional DB overhead may be a concern. - Add volume_history_enabled option to cinder/common/config.py (default: True) - Update _record_volume_history() to check config before recording - Add test for disabled config behavior - Update SAP documentation with configuration details Change-Id: I066aa79e4e2538831bbe152007c84ec23bdc9866
3b610af to
f75b318
Compare
Change-Id: I118f4ea376857bcd5bd740e62f06106c253a4fcc
The volume_update hook was not recording changes for operations that go through Volume.save() (extend, rename, etc.) because SQLAlchemy's ORM reflects the UPDATE back onto the model object within the same session. Fix by copying the relevant old values into a dictionary BEFORE the query.filter_by().update() call, so we compare against the true old values rather than the post-update values. Also adds datetime serialization handling for proper JSON storage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a volume history tracking feature that records every mutation to a volume's DB row in a new
volume_historytable. Each history record captures JSON deltas of old/new values along with contextual metadata.Status: Work In Progress
Changes
VolumeHistorymodel inmodels.pywith FK tovolumes.id633b14d87cecto create thevolume_historytablevolume_create(),volume_update(),volume_destroy(),volume_attached(),volume_detached()volume_history_get_all_by_volume()to retrieve historyvolume_history_enabledto disable tracking in high-throughput environmentssap-doc/volume-history.mdConfiguration
Volume history tracking is enabled by default but can be disabled:
Tracked Actions
createupdatedestroyattachdetachPerformance Overhead Analysis
volume_update()— Heaviest overhead (most frequent operation)SELECTquery (query.filter_by(id=volume_id).first())json.dumps()of changes dictsession.add()for history INSERTEach
volume_updateadds 1 extra SELECT by PK + 1 extra INSERT, both within the same DB transaction that was already open. The SELECT is a fast indexed lookup on thevolumes.idprimary key.volume_destroy()— Moderate overheadquery.first()to get current statussession.add()for history INSERTAdds 1 extra SELECT + 1 extra INSERT.
volume_create()— Minimal overheadsession.add()for history INSERTJust 1 extra INSERT, no extra SELECT since we already have the values dict.
volume_attached()/volume_detached()— Minimal overheadSame as create — just 1 extra INSERT each, no extra SELECT.
Mitigation
Set
volume_history_enabled = Falseincinder.confto completely disable history tracking and eliminate all overhead.Testing
All tests pass in Docker (Python 3.10):
test_volume_destroy_deletes_dependent_data)TODO