Optionally delete commands in mysql storage backend #48
Conversation
jessepeterson
force-pushed
the
mysql-command-delete
branch
from
0aaa0d0
to
37e877e
Compare
jessepeterson
force-pushed
the
mysql-command-delete
branch
from
37e877e
to
d9756d0
Compare
| DELETE | ||
| c | ||
| FROM | ||
| commands AS c | ||
| LEFT JOIN enrollment_queue AS q | ||
| ON q.command_uuid = c.command_uuid | ||
| LEFT JOIN command_results AS r | ||
| ON r.command_uuid = c.command_uuid | ||
| WHERE | ||
| c.command_uuid = ? AND | ||
| q.command_uuid IS NULL AND | ||
| r.command_uuid IS NULL; |
There was a problem hiding this comment.
There was a problem hiding this comment.
"Delete from 'commands' where the command_uuid matches the provided one, and there is no matching record in the enrollment queue, and there is no matching record in the results table."
After you have a delete that translates to:
"Delete from the enrollment queue and the results table where the id in the queue matches the provided value and the command uuid matches the provided value. It's ok if there is nothing in the results table"
So, combined:
"For command_uuid=?, After I delete everything associated with id=? clean up the commands entry if it refers to nothing".
And we're relying on command_uuid being unique. Seems good?
There was a problem hiding this comment.
I'm a bit worried on the multi table delete used here again, due to the intricacies of its failure/ordering.
when you could always do some classic SQL command like
DELETE c
FROM commands
WHERE NOT EXISTS (SELECT * from enrollment_queue WHERE command_uuid = c.command_uuid)
AND NOT EXISTS (SELECT * from command_results WHERE command_uuid = c.command_uuid)but since we're only deleting one row here it probably won't matter :)
| DELETE | ||
| c | ||
| FROM | ||
| commands AS c | ||
| LEFT JOIN enrollment_queue AS q | ||
| ON q.command_uuid = c.command_uuid | ||
| LEFT JOIN command_results AS r | ||
| ON r.command_uuid = c.command_uuid | ||
| WHERE | ||
| c.command_uuid = ? AND | ||
| q.command_uuid IS NULL AND | ||
| r.command_uuid IS NULL; |
There was a problem hiding this comment.
I'm a bit worried on the multi table delete used here again, due to the intricacies of its failure/ordering.
when you could always do some classic SQL command like
DELETE c
FROM commands
WHERE NOT EXISTS (SELECT * from enrollment_queue WHERE command_uuid = c.command_uuid)
AND NOT EXISTS (SELECT * from command_results WHERE command_uuid = c.command_uuid)but since we're only deleting one row here it probably won't matter :)
| } | ||
| return tx.Commit() | ||
| } | ||
|
|
||
| func (s *MySQLStorage) StoreCommandReport(r *mdm.Request, result *mdm.CommandResults) error { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
NanoMDM allows for a single command to be queued for multiple devices. Since micromdm#48 landed, both queued commands and the command data itself is deleted after a device in finished with a command. However, I have observed deadlocks which occur when multiple devices trigger deletion for a shared command at once. This can be avoided by placing a record lock on the command row before attempting a delete, so that only one device's enrollment queue can be cleared at a time. This should have minimal impacts on performance as it will only serialize deletes to a single command. Furthermore, as the lock on the command record must be taken anyway, this does not add any additional locking to the transaction. Testing: I have tested this, and have confirmed that it prevents deadlocks in NanoMDM. Furthermore, I have tested that commands are still delivered and responded to.
From the changes to the operations guide:
This implements the feature discussed in #43 for the
mysqlstorage backend.