Dual Storage Architecture #6
Conversation
* Dual writer: mode 3 * Add integration tests for playlits in mode 3 * Remove todo * Update pkg/apiserver/rest/dualwriter_mode3.go Co-authored-by: Arati R. <[email protected]> * Admin: Fixes an issue where user accounts could not be enabled (#88117) Fix: unable to enable user * [REVIEW] FInish mode 3 and add tests * Improve logging * Update dependencies * Update pkg/apiserver/rest/dualwriter_mode3_test.go Co-authored-by: maicon <[email protected]> * remove test assertion * Use mode log when dual writer is initiated --------- Co-authored-by: Arati R. <[email protected]> Co-authored-by: gonvee <[email protected]> Co-authored-by: maicon <[email protected]>
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bugbot free trial expires on August 11, 2025
Learn more in the Cursor dashboard.
| created, err := d.Storage.Create(ctx, obj, createValidation, options) | ||
| if err != nil { | ||
| log.Error(err, "unable to create object in storage") | ||
| d.recordLegacyDuration(true, mode3Str, options.Kind, method, startStorage) |
There was a problem hiding this comment.
Bug: Storage Failures Misattributed to Legacy Metrics
When Storage.Create fails in DualWriterMode3.Create, the d.recordLegacyDuration method is incorrectly called to record the failure, instead of d.recordStorageDuration. This misattributes storage operation failures to legacy storage metrics.
Locations (2)
| ctx, cancel := context.WithTimeoutCause(ctx, time.Second*10, errors.New("legacy deletecollection timeout")) | ||
| defer cancel() | ||
| _, err := d.Legacy.DeleteCollection(ctx, deleteValidation, options, listOptions) | ||
| d.recordStorageDuration(err != nil, mode3Str, options.Kind, method, startLegacy) |
There was a problem hiding this comment.
Bug: Metric Recording Error in DeleteCollection
In pkg/apiserver/rest/dualwriter_mode3.go, the DeleteCollection method incorrectly records metrics for the legacy store operation. Inside the goroutine handling the legacy DeleteCollection, d.recordStorageDuration is called instead of d.recordLegacyDuration, leading to legacy operation durations being misattributed as storage durations.
Locations (1)
| d.recordStorageDuration(true, mode3Str, options.Kind, method, startStorage) | ||
| return res, async, err | ||
| } | ||
| d.recordStorageDuration(false, mode3Str, name, method, startStorage) |
There was a problem hiding this comment.
Bug: Incorrect Parameter in Metric Recording
The Delete method in DualWriterMode3 incorrectly passes name instead of options.Kind as the third parameter to recordStorageDuration. This is inconsistent with other metric recording calls in the file and leads to incorrect metrics categorization.
Locations (1)
| upstream: objInfo, | ||
| updated: updated, | ||
| log.Error(err, "unable to update in storage") | ||
| d.recordLegacyDuration(true, mode3Str, options.Kind, method, startStorage) |
There was a problem hiding this comment.
Bug: Incorrect Metric Recording in DualWriterMode3
The DualWriterMode3.Update method incorrectly records metrics for Storage.Update() failures by calling d.recordLegacyDuration() instead of d.recordStorageDuration().
Locations (1)
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had any further activity in the last 2 weeks. Thank you for your contributions! |
3 participants
Test 6