diff --git a/changes/44189-host-profile-perf b/changes/44189-host-profile-perf new file mode 100644 index 0000000000..e716cf4bd9 --- /dev/null +++ b/changes/44189-host-profile-perf @@ -0,0 +1 @@ +* Improved Windows MDM profile removal performance by skipping redundant database writes for verified-remove ACKs. \ No newline at end of file diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index 24b890fa41..a2ba926f29 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -822,9 +822,10 @@ func updateMDMWindowsHostProfileStatusFromResponseDB( return ctxerr.Wrap(ctx, err, "running query to get matching profiles") } - // batch-update the matching entries with the desired detail and status + // Partition matching entries into upsert and delete buckets. var sb strings.Builder args = args[:0] + var deleteCommandUUIDs []string for _, hp := range matchingHostProfiles { payload := uuidsToPayloads[hp.CommandUUID] if payload.Status != nil && *payload.Status == fleet.MDMDeliveryFailed { @@ -839,32 +840,42 @@ func updateMDMWindowsHostProfileStatusFromResponseDB( hp.Retries++ } } + + // Delete bucket: remove operations that resolved to a terminal state. + // Removes are best-effort; both verified and failed are terminal since + // failed removes are non-retryable and should not surface as host-level + // failures in profile summaries. + if hp.OperationType == fleet.MDMOperationTypeRemove && payload.Status != nil && + (*payload.Status == fleet.MDMDeliveryVerified || *payload.Status == fleet.MDMDeliveryFailed) { + deleteCommandUUIDs = append(deleteCommandUUIDs, hp.CommandUUID) + continue + } + args = append(args, hp.HostUUID, hp.ProfileUUID, payload.Detail, payload.Status, hp.Retries, hp.Checksum) sb.WriteString("(?, ?, ?, ?, ?, command_uuid, ?),") } + // Execute batched UPSERT for the upsert bucket. values := strings.TrimSuffix(sb.String(), ",") - if len(values) == 0 { - return nil - } - stmt = fmt.Sprintf(updateHostProfilesStmt, values) - if _, err = tx.ExecContext(ctx, stmt, args...); err != nil { - return ctxerr.Wrap(ctx, err, "updating host profiles") + if len(values) > 0 { + stmt = fmt.Sprintf(updateHostProfilesStmt, values) + if _, err = tx.ExecContext(ctx, stmt, args...); err != nil { + return ctxerr.Wrap(ctx, err, "updating host profiles") + } } - // Clean up remove + verified rows for the command UUIDs we just processed. - // Only delete 'verified' (not 'verifying'); verifying is an in-flight - // state and should not be deleted until the device confirms. We scope to - // specific command_uuids to avoid deleting rows from concurrent responses. - removeCleanupStmt, removeCleanupArgs, err := sqlx.In(` - DELETE FROM host_mdm_windows_profiles - WHERE host_uuid = ? AND command_uuid IN (?) AND operation_type = ? AND status = ?`, - hostUUID, commandUUIDs, fleet.MDMOperationTypeRemove, fleet.MDMDeliveryVerified) - if err != nil { - return ctxerr.Wrap(ctx, err, "building IN for remove cleanup") - } - if _, err = tx.ExecContext(ctx, removeCleanupStmt, removeCleanupArgs...); err != nil { - return ctxerr.Wrap(ctx, err, "cleaning up completed remove profiles") + // Execute batched DELETE for terminal remove operations. + if len(deleteCommandUUIDs) > 0 { + deleteStmt, deleteArgs, err := sqlx.In(` + DELETE FROM host_mdm_windows_profiles + WHERE host_uuid = ? AND command_uuid IN (?)`, + hostUUID, deleteCommandUUIDs) + if err != nil { + return ctxerr.Wrap(ctx, err, "building IN for remove cleanup") + } + if _, err = tx.ExecContext(ctx, deleteStmt, deleteArgs...); err != nil { + return ctxerr.Wrap(ctx, err, "cleaning up completed remove profiles") + } } return nil diff --git a/server/datastore/mysql/microsoft_mdm_test.go b/server/datastore/mysql/microsoft_mdm_test.go index 3cc16789d8..993b84c6f2 100644 --- a/server/datastore/mysql/microsoft_mdm_test.go +++ b/server/datastore/mysql/microsoft_mdm_test.go @@ -3651,6 +3651,228 @@ WHERE host_uuid = ? AND command_uuid = ?`, enrolledDevice1.HostUUID, replaceCmd. }) }) + t.Run("remove status outcomes", func(t *testing.T) { + // Both verified and failed removes are terminal (best-effort removal) + // and should be deleted from host_mdm_windows_profiles. + cases := []struct { + name string + statusCode int + }{ + {"verified remove deletes row", 200}, + {"failed remove deletes row", 418}, // not in the "treated as success" list, maps to Failed + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + deleteCommandUUID := uuid.NewString() + cmd := &fleet.MDMWindowsCommand{ + CommandUUID: deleteCommandUUID, + RawCommand: fmt.Appendf([]byte{}, ` + + %s + + + ./Device/Vendor/MSFT/Policy/Config/System/DisableOneDriveFileSync + + + +`, deleteCommandUUID), + } + err := ds.mdmWindowsInsertCommandForHostsDB(t.Context(), ds.primary, + []string{enrolledDevice1.MDMDeviceID}, cmd) + require.NoError(t, err) + + profileUUID := uuid.NewString() + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(t.Context(), ` +INSERT INTO host_mdm_windows_profiles (host_uuid, status, operation_type, command_uuid, profile_name, profile_uuid) +VALUES (?, 'verifying', 'remove', ?, 'disable-onedrive', ?)`, enrolledDevice1.HostUUID, deleteCommandUUID, profileUUID) + return err + }) + + enrichedSyncML := createResponseAsEnrichedSyncML(t, enrolledDevice1, []enrichResponseEntry{ + {Type: "Delete", StatusCode: tc.statusCode, UUID: deleteCommandUUID}, + }) + _, err = ds.MDMWindowsSaveResponse(t.Context(), enrolledDevice1, enrichedSyncML, []string{}) + require.NoError(t, err) + + var count int + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(t.Context(), q, &count, ` +SELECT COUNT(*) FROM host_mdm_windows_profiles +WHERE host_uuid = ? AND command_uuid = ?`, enrolledDevice1.HostUUID, deleteCommandUUID) + }) + assert.Equal(t, 0, count, "terminal remove row should be deleted") + }) + } + }) + + t.Run("mixed install and remove in same batch", func(t *testing.T) { + // Install profile (Replace command, status 200) + Remove profile (Delete command, status 200). + replaceCommandUUID := uuid.NewString() + replaceCmd := &fleet.MDMWindowsCommand{ + CommandUUID: replaceCommandUUID, + RawCommand: fmt.Appendf([]byte{}, ` + + %s + + + ./Device/Vendor/MSFT/Policy/Config/System/DisableOneDriveFileSync + + int + 1 + + +`, replaceCommandUUID), + TargetLocURI: "", + } + deleteCommandUUID := uuid.NewString() + deleteCmd := &fleet.MDMWindowsCommand{ + CommandUUID: deleteCommandUUID, + RawCommand: fmt.Appendf([]byte{}, ` + + %s + + + ./Device/Vendor/MSFT/Policy/Config/System/SomeOtherSetting + + + +`, deleteCommandUUID), + TargetLocURI: "", + } + + err := ds.mdmWindowsInsertCommandForHostsDB(t.Context(), ds.primary, + []string{enrolledDevice1.MDMDeviceID}, replaceCmd) + require.NoError(t, err) + err = ds.mdmWindowsInsertCommandForHostsDB(t.Context(), ds.primary, + []string{enrolledDevice1.MDMDeviceID}, deleteCmd) + require.NoError(t, err) + + installProfileUUID := uuid.NewString() + removeProfileUUID := uuid.NewString() + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(t.Context(), ` +INSERT INTO host_mdm_windows_profiles (host_uuid, status, operation_type, command_uuid, profile_name, profile_uuid) +VALUES (?, 'pending', 'install', ?, 'install-profile', ?)`, enrolledDevice1.HostUUID, replaceCommandUUID, installProfileUUID) + require.NoError(t, err) + _, err = q.ExecContext(t.Context(), ` +INSERT INTO host_mdm_windows_profiles (host_uuid, status, operation_type, command_uuid, profile_name, profile_uuid) +VALUES (?, 'verifying', 'remove', ?, 'remove-profile', ?)`, enrolledDevice1.HostUUID, deleteCommandUUID, removeProfileUUID) + return err + }) + + cmdEntries := []enrichResponseEntry{ + {Type: "Replace", StatusCode: 200, UUID: replaceCommandUUID}, + {Type: "Delete", StatusCode: 200, UUID: deleteCommandUUID}, + } + enrichedSyncML := createResponseAsEnrichedSyncML(t, enrolledDevice1, cmdEntries) + _, err = ds.MDMWindowsSaveResponse(t.Context(), enrolledDevice1, enrichedSyncML, []string{}) + require.NoError(t, err) + + // Install profile should be upserted with verified status. + var installStatus string + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(t.Context(), q, &installStatus, ` +SELECT status FROM host_mdm_windows_profiles +WHERE host_uuid = ? AND command_uuid = ?`, enrolledDevice1.HostUUID, replaceCommandUUID) + }) + assert.Equal(t, "verified", installStatus, "install profile should be verified") + + // Remove profile should be deleted. + var removeCount int + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(t.Context(), q, &removeCount, ` +SELECT COUNT(*) FROM host_mdm_windows_profiles +WHERE host_uuid = ? AND command_uuid = ?`, enrolledDevice1.HostUUID, deleteCommandUUID) + }) + assert.Equal(t, 0, removeCount, "verified remove row should be deleted") + }) + + t.Run("remove then reinstall same profile", func(t *testing.T) { + // Validates that deleting a verified-remove row by command_uuid does not + // interfere with a fresh install of the same (host_uuid, profile_uuid) pair. + profileUUID := uuid.NewString() + + // Step 1: create a remove row and ACK it as verified → row deleted. + removeCommandUUID := uuid.NewString() + removeCmd := &fleet.MDMWindowsCommand{ + CommandUUID: removeCommandUUID, + RawCommand: fmt.Appendf([]byte{}, ` + + %s + + + ./Device/Vendor/MSFT/Policy/Config/System/ReinstallTest + + + +`, removeCommandUUID), + } + err := ds.mdmWindowsInsertCommandForHostsDB(t.Context(), ds.primary, + []string{enrolledDevice1.MDMDeviceID}, removeCmd) + require.NoError(t, err) + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(t.Context(), ` +INSERT INTO host_mdm_windows_profiles (host_uuid, status, operation_type, command_uuid, profile_name, profile_uuid) +VALUES (?, 'verifying', 'remove', ?, 'reinstall-test', ?)`, enrolledDevice1.HostUUID, removeCommandUUID, profileUUID) + return err + }) + enrichedSyncML := createResponseAsEnrichedSyncML(t, enrolledDevice1, []enrichResponseEntry{ + {Type: "Delete", StatusCode: 200, UUID: removeCommandUUID}, + }) + _, err = ds.MDMWindowsSaveResponse(t.Context(), enrolledDevice1, enrichedSyncML, []string{}) + require.NoError(t, err) + + var count int + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(t.Context(), q, &count, ` +SELECT COUNT(*) FROM host_mdm_windows_profiles +WHERE host_uuid = ? AND profile_uuid = ?`, enrolledDevice1.HostUUID, profileUUID) + }) + require.Equal(t, 0, count, "remove row should be deleted after verified ACK") + + // Step 2: fresh install of the same profile with a new command UUID. + installCommandUUID := uuid.NewString() + installCmd := &fleet.MDMWindowsCommand{ + CommandUUID: installCommandUUID, + RawCommand: fmt.Appendf([]byte{}, ` + + %s + + + ./Device/Vendor/MSFT/Policy/Config/System/ReinstallTest + + int + 1 + + +`, installCommandUUID), + } + err = ds.mdmWindowsInsertCommandForHostsDB(t.Context(), ds.primary, + []string{enrolledDevice1.MDMDeviceID}, installCmd) + require.NoError(t, err) + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(t.Context(), ` +INSERT INTO host_mdm_windows_profiles (host_uuid, status, operation_type, command_uuid, profile_name, profile_uuid) +VALUES (?, 'pending', 'install', ?, 'reinstall-test', ?)`, enrolledDevice1.HostUUID, installCommandUUID, profileUUID) + return err + }) + enrichedSyncML = createResponseAsEnrichedSyncML(t, enrolledDevice1, []enrichResponseEntry{ + {Type: "Replace", StatusCode: 200, UUID: installCommandUUID}, + }) + _, err = ds.MDMWindowsSaveResponse(t.Context(), enrolledDevice1, enrichedSyncML, []string{}) + require.NoError(t, err) + + // The reinstalled profile should land as verified. + var status string + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(t.Context(), q, &status, ` +SELECT status FROM host_mdm_windows_profiles +WHERE host_uuid = ? AND profile_uuid = ?`, enrolledDevice1.HostUUID, profileUUID) + }) + assert.Equal(t, "verified", status, "reinstalled profile should be verified") + }) + t.Run("wipe failure returns WipeFailed result", func(t *testing.T) { ctx := t.Context()