Skip to content

Commit b5aafbb

Browse files
Jiale Zhangjialez0
authored andcommitted
Fix AA instance heartbeat deduplication
Enforce the AA heartbeat instance_id uniqueness migration and clean up historical duplicate heartbeat rows before creating the index. Make active instance listing resilient to stale duplicates, and update heartbeat upserts to refresh the full instance metadata including IP and EAS fields. Signed-off-by: Jiale Zhang <xinjian.zjl@alibaba-inc.com>
1 parent 8a2a7a8 commit b5aafbb

4 files changed

Lines changed: 233 additions & 16 deletions

File tree

trustee-gateway/internal/persistence/repository/aa_repository.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ func (r *AAInstanceRepository) UpsertHeartbeat(heartbeat *models.AAInstanceHeart
3333
"image_id",
3434
"instance_name",
3535
"owner_account_id",
36+
"eas_model_id",
37+
"eas_instance_id",
38+
"eas_pod_name",
39+
"ip",
3640
"client_ip",
3741
"last_heartbeat",
3842
"updated_at",
@@ -43,8 +47,22 @@ func (r *AAInstanceRepository) UpsertHeartbeat(heartbeat *models.AAInstanceHeart
4347
// GetActiveHeartbeats retrieves all heartbeats that are newer than the cutoff time
4448
func (r *AAInstanceRepository) GetActiveHeartbeats(cutoffTime time.Time) ([]models.AAInstanceHeartbeat, error) {
4549
var heartbeats []models.AAInstanceHeartbeat
46-
err := r.db.Where("last_heartbeat >= ?", cutoffTime).Order("last_heartbeat desc").Find(&heartbeats).Error
47-
return heartbeats, err
50+
err := r.db.Where("last_heartbeat >= ?", cutoffTime).Order("last_heartbeat desc, id desc").Find(&heartbeats).Error
51+
if err != nil {
52+
return nil, err
53+
}
54+
55+
seen := make(map[string]struct{}, len(heartbeats))
56+
deduped := make([]models.AAInstanceHeartbeat, 0, len(heartbeats))
57+
for _, heartbeat := range heartbeats {
58+
if _, ok := seen[heartbeat.InstanceID]; ok {
59+
continue
60+
}
61+
seen[heartbeat.InstanceID] = struct{}{}
62+
deduped = append(deduped, heartbeat)
63+
}
64+
65+
return deduped, nil
4866
}
4967

5068
// CleanupExpiredHeartbeats removes heartbeat records older than the cutoff time
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package repository
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/openanolis/trustee/gateway/internal/models"
8+
"github.com/openanolis/trustee/gateway/internal/persistence/storage"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"gorm.io/driver/sqlite"
12+
"gorm.io/gorm"
13+
)
14+
15+
func setupAAInstanceTestDB(t *testing.T, withUniqueIndex bool) *storage.Database {
16+
t.Helper()
17+
18+
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
19+
require.NoError(t, err)
20+
21+
err = db.AutoMigrate(&models.AAInstanceHeartbeat{})
22+
require.NoError(t, err)
23+
24+
if withUniqueIndex {
25+
err = db.Exec("CREATE UNIQUE INDEX idx_aa_heartbeat_instance_id ON aa_instance_heartbeats(instance_id)").Error
26+
require.NoError(t, err)
27+
}
28+
29+
return &storage.Database{DB: db}
30+
}
31+
32+
func TestAAInstanceRepositoryUpsertHeartbeatUpdatesExistingRecord(t *testing.T) {
33+
repo := NewAAInstanceRepository(setupAAInstanceTestDB(t, true))
34+
35+
err := repo.UpsertHeartbeat(&models.AAInstanceHeartbeat{
36+
InstanceInfo: models.InstanceInfo{
37+
InstanceID: "i-test",
38+
ImageID: "old-image",
39+
InstanceName: "old-name",
40+
OwnerAccountID: "old-owner",
41+
IP: "10.0.0.1",
42+
},
43+
ClientIP: "10.0.0.1",
44+
})
45+
require.NoError(t, err)
46+
47+
err = repo.UpsertHeartbeat(&models.AAInstanceHeartbeat{
48+
InstanceInfo: models.InstanceInfo{
49+
InstanceID: "i-test",
50+
ImageID: "new-image",
51+
InstanceName: "new-name",
52+
OwnerAccountID: "new-owner",
53+
EasModelID: "model-1",
54+
EasInstanceID: "eas-1",
55+
EasPodName: "pod-1",
56+
IP: "10.0.0.2",
57+
},
58+
ClientIP: "10.0.0.2",
59+
})
60+
require.NoError(t, err)
61+
62+
var heartbeats []models.AAInstanceHeartbeat
63+
err = repo.db.Find(&heartbeats).Error
64+
require.NoError(t, err)
65+
require.Len(t, heartbeats, 1)
66+
67+
heartbeat := heartbeats[0]
68+
assert.Equal(t, "i-test", heartbeat.InstanceID)
69+
assert.Equal(t, "new-image", heartbeat.ImageID)
70+
assert.Equal(t, "new-name", heartbeat.InstanceName)
71+
assert.Equal(t, "new-owner", heartbeat.OwnerAccountID)
72+
assert.Equal(t, "model-1", heartbeat.EasModelID)
73+
assert.Equal(t, "eas-1", heartbeat.EasInstanceID)
74+
assert.Equal(t, "pod-1", heartbeat.EasPodName)
75+
assert.Equal(t, "10.0.0.2", heartbeat.IP)
76+
assert.Equal(t, "10.0.0.2", heartbeat.ClientIP)
77+
}
78+
79+
func TestAAInstanceRepositoryGetActiveHeartbeatsDeduplicatesInstanceID(t *testing.T) {
80+
repo := NewAAInstanceRepository(setupAAInstanceTestDB(t, false))
81+
now := time.Now()
82+
83+
records := []models.AAInstanceHeartbeat{
84+
{
85+
InstanceInfo: models.InstanceInfo{InstanceID: "i-test", IP: "10.0.0.1"},
86+
ClientIP: "10.0.0.1",
87+
LastHeartbeat: now.Add(-time.Minute),
88+
},
89+
{
90+
InstanceInfo: models.InstanceInfo{InstanceID: "i-test", IP: "10.0.0.2"},
91+
ClientIP: "10.0.0.2",
92+
LastHeartbeat: now,
93+
},
94+
{
95+
InstanceInfo: models.InstanceInfo{InstanceID: "i-other", IP: "10.0.0.3"},
96+
ClientIP: "10.0.0.3",
97+
LastHeartbeat: now.Add(-30 * time.Second),
98+
},
99+
}
100+
require.NoError(t, repo.db.Create(&records).Error)
101+
102+
heartbeats, err := repo.GetActiveHeartbeats(now.Add(-10 * time.Minute))
103+
require.NoError(t, err)
104+
require.Len(t, heartbeats, 2)
105+
106+
assert.Equal(t, "i-test", heartbeats[0].InstanceID)
107+
assert.Equal(t, "10.0.0.2", heartbeats[0].ClientIP)
108+
assert.Equal(t, "i-other", heartbeats[1].InstanceID)
109+
}

trustee-gateway/internal/persistence/storage/database.go

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,14 @@ func (d *Database) migrateSchema() error {
167167
// This prevents duplicate heartbeat records for the same instance
168168
if d.config.Type == "mysql" {
169169
if err := d.migrateAAInstanceHeartbeatIndexMySQL(); err != nil {
170-
logrus.Warnf("Failed to migrate AAInstanceHeartbeat index: %v", err)
170+
return fmt.Errorf("failed to migrate AAInstanceHeartbeat index: %w", err)
171171
}
172172
} else {
173173
// SQLite - file-based SQLite has built-in file locking
174174
// In-memory SQLite is per-process, so no cross-process conflict
175-
d.migrateAAInstanceHeartbeatIndexSQLite()
175+
if err := d.migrateAAInstanceHeartbeatIndexSQLite(); err != nil {
176+
return fmt.Errorf("failed to migrate AAInstanceHeartbeat index: %w", err)
177+
}
176178
}
177179

178180
return nil
@@ -190,8 +192,7 @@ func (d *Database) migrateAAInstanceHeartbeatIndexMySQL() error {
190192
}
191193

192194
if lockResult != 1 {
193-
logrus.Info("Another instance is running migration, skipping")
194-
return nil
195+
return fmt.Errorf("failed to acquire migration lock: another instance is running migration")
195196
}
196197

197198
// Ensure lock is released when done
@@ -213,18 +214,26 @@ func (d *Database) migrateAAInstanceHeartbeatIndexMySQL() error {
213214

214215
// Alter column to VARCHAR(255) for index compatibility
215216
if err := d.DB.Exec("ALTER TABLE aa_instance_heartbeats MODIFY instance_id VARCHAR(255)").Error; err != nil {
216-
logrus.Warnf("Failed to alter instance_id column: %v", err)
217+
return fmt.Errorf("failed to alter instance_id column: %w", err)
218+
}
219+
220+
if err := d.DB.Exec("DELETE FROM aa_instance_heartbeats WHERE instance_id IS NULL OR instance_id = ''").Error; err != nil {
221+
return fmt.Errorf("failed to clean up empty instance_id records: %w", err)
217222
}
218223

219224
// Clean up duplicate instance_id records before creating unique index
220-
// Keep only the latest record (by id) for each instance_id
225+
// Keep only the latest heartbeat for each instance_id.
221226
cleanupSQL := `
222227
DELETE t1 FROM aa_instance_heartbeats t1
223228
INNER JOIN aa_instance_heartbeats t2
224-
WHERE t1.instance_id = t2.instance_id AND t1.id < t2.id
229+
WHERE t1.instance_id = t2.instance_id
230+
AND (
231+
t1.last_heartbeat < t2.last_heartbeat
232+
OR (t1.last_heartbeat = t2.last_heartbeat AND t1.id < t2.id)
233+
)
225234
`
226235
if err := d.DB.Exec(cleanupSQL).Error; err != nil {
227-
logrus.Warnf("Failed to clean up duplicate instance_id records: %v", err)
236+
return fmt.Errorf("failed to clean up duplicate instance_id records: %w", err)
228237
} else {
229238
logrus.Info("Cleaned up duplicate instance_id records in aa_instance_heartbeats")
230239
}
@@ -238,29 +247,45 @@ func (d *Database) migrateAAInstanceHeartbeatIndexMySQL() error {
238247
}
239248

240249
// migrateAAInstanceHeartbeatIndexSQLite handles unique index creation for SQLite
241-
func (d *Database) migrateAAInstanceHeartbeatIndexSQLite() {
250+
func (d *Database) migrateAAInstanceHeartbeatIndexSQLite() error {
242251
migrator := d.DB.Migrator()
243252
if migrator.HasIndex(&models.AAInstanceHeartbeat{}, "idx_aa_heartbeat_instance_id") {
244-
return
253+
return nil
254+
}
255+
256+
if err := d.DB.Exec("DELETE FROM aa_instance_heartbeats WHERE instance_id IS NULL OR instance_id = ''").Error; err != nil {
257+
return fmt.Errorf("failed to clean up empty instance_id records: %w", err)
245258
}
246259

247260
// Clean up duplicate instance_id records before creating unique index
248-
// Keep only the latest record (by id) for each instance_id
261+
// Keep only the latest heartbeat for each instance_id.
249262
cleanupSQL := `
250263
DELETE FROM aa_instance_heartbeats
251264
WHERE id NOT IN (
252-
SELECT MAX(id) FROM aa_instance_heartbeats GROUP BY instance_id
265+
SELECT id FROM (
266+
SELECT h1.id
267+
FROM aa_instance_heartbeats h1
268+
LEFT JOIN aa_instance_heartbeats h2
269+
ON h1.instance_id = h2.instance_id
270+
AND (
271+
h1.last_heartbeat < h2.last_heartbeat
272+
OR (h1.last_heartbeat = h2.last_heartbeat AND h1.id < h2.id)
273+
)
274+
WHERE h2.id IS NULL
275+
)
253276
)
254277
`
255278
if err := d.DB.Exec(cleanupSQL).Error; err != nil {
256-
logrus.Warnf("Failed to clean up duplicate instance_id records: %v", err)
279+
return fmt.Errorf("failed to clean up duplicate instance_id records: %w", err)
257280
} else {
258281
logrus.Info("Cleaned up duplicate instance_id records in aa_instance_heartbeats")
259282
}
260283

261284
if err := d.DB.Exec("CREATE UNIQUE INDEX IF NOT EXISTS idx_aa_heartbeat_instance_id ON aa_instance_heartbeats(instance_id)").Error; err != nil {
262-
logrus.Warnf("Failed to create unique index on instance_id: %v", err)
285+
return fmt.Errorf("failed to create unique index on instance_id: %w", err)
263286
}
287+
288+
return nil
264289
}
265290

266291
// restoreFromBackup restores data from backup file to in-memory database using native backup API
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package storage
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/openanolis/trustee/gateway/internal/config"
8+
"github.com/openanolis/trustee/gateway/internal/models"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"gorm.io/driver/sqlite"
12+
"gorm.io/gorm"
13+
)
14+
15+
func TestMigrateAAInstanceHeartbeatIndexSQLiteCleansDuplicates(t *testing.T) {
16+
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
17+
require.NoError(t, err)
18+
19+
require.NoError(t, db.AutoMigrate(&models.AAInstanceHeartbeat{}))
20+
21+
now := time.Now()
22+
records := []models.AAInstanceHeartbeat{
23+
{
24+
InstanceInfo: models.InstanceInfo{InstanceID: "i-test", IP: "10.0.0.1"},
25+
ClientIP: "10.0.0.1",
26+
LastHeartbeat: now.Add(-time.Minute),
27+
},
28+
{
29+
InstanceInfo: models.InstanceInfo{InstanceID: "i-test", IP: "10.0.0.2"},
30+
ClientIP: "10.0.0.2",
31+
LastHeartbeat: now,
32+
},
33+
{
34+
InstanceInfo: models.InstanceInfo{InstanceID: ""},
35+
ClientIP: "10.0.0.3",
36+
LastHeartbeat: now,
37+
},
38+
}
39+
require.NoError(t, db.Create(&records).Error)
40+
41+
database := &Database{
42+
DB: db,
43+
config: &config.DatabaseConfig{Type: "sqlite"},
44+
}
45+
require.NoError(t, database.migrateAAInstanceHeartbeatIndexSQLite())
46+
47+
var heartbeats []models.AAInstanceHeartbeat
48+
require.NoError(t, db.Order("last_heartbeat desc").Find(&heartbeats).Error)
49+
require.Len(t, heartbeats, 1)
50+
assert.Equal(t, "i-test", heartbeats[0].InstanceID)
51+
assert.Equal(t, "10.0.0.2", heartbeats[0].ClientIP)
52+
53+
require.NoError(t, db.Create(&models.AAInstanceHeartbeat{
54+
InstanceInfo: models.InstanceInfo{InstanceID: "i-other", IP: "10.0.0.4"},
55+
ClientIP: "10.0.0.4",
56+
LastHeartbeat: now,
57+
}).Error)
58+
59+
err = db.Create(&models.AAInstanceHeartbeat{
60+
InstanceInfo: models.InstanceInfo{InstanceID: "i-test", IP: "10.0.0.5"},
61+
ClientIP: "10.0.0.5",
62+
LastHeartbeat: now,
63+
}).Error
64+
assert.Error(t, err)
65+
}

0 commit comments

Comments
 (0)