Alpha162 commented on issue #13399: URL: https://github.com/apache/cloudstack/issues/13399#issuecomment-5325282390
@DaanHoogland thanks, I'll go with option 1 on your idempotency point. Both `` `cloud`.`IDEMPOTENT_DROP_UNIQUE_KEY` `` and `` `cloud`.`IDEMPOTENT_ADD_UNIQUE_KEY` `` already carry `CONTINUE HANDLER`s (`1091, 1025` and `1061` respectively), so a re-run is a no-op in effect and it needs no new procedures. Option 3 is out for exactly the reason you give, the plain `ALTER TABLE ... ADD UNIQUE KEY` isn't idempotent and would fail on a second pass. It also follows the existing precedent in `schema-42100to42200.sql` (lines 88-89), which drops and re-adds `uc_counter__provider__source__value` the same way. Worth noting for the rollback concern specifically: widening `(volume_id, created)` to `(volume_id, created, vm_id)` is strictly *less* restrictive, so no existing row can violate the new key. The DDL can't fail on data, only on a key-name mismatch, which is the caveat I flagged above. I'll target `main` (`schema-42210to42300.sql`). Say the word if you'd like a `4.22` backport too and I'll open a second PR against a new `schema-42210to42220.sql`. And yes, I'll raise the two subissues: 1. `GenericDaoBase.persist()` leaves the caller's transaction unbalanced when an insert throws 2. `UsageManagerImpl.parse()` rewind to the oldest unprocessed usage event is unbounded I'll link them back here once created. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
