From 83032ba500cf1660fe919cb12632fb0fda6e7bbc Mon Sep 17 00:00:00 2001 From: Hongyu Zhou Date: Thu, 2 Mar 2023 15:59:22 -0500 Subject: [PATCH 1/4] lift metric outside of the isECS check --- pkg/ledger/ledger_monitor.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/ledger/ledger_monitor.go b/pkg/ledger/ledger_monitor.go index f0dff297..6a2e3ff3 100644 --- a/pkg/ledger/ledger_monitor.go +++ b/pkg/ledger/ledger_monitor.go @@ -95,14 +95,14 @@ func (m *Monitor) Start(ctx context.Context) { } health = pointer.ToBool(false) } - switch { - case health == nil: - stats.Set("ledger-health", 1, stats.T("status", "unknown")) - case *health == false: - stats.Set("ledger-health", 1, stats.T("status", "unhealthy")) - case *health == true: - stats.Set("ledger-health", 1, stats.T("status", "healthy")) - } + } + switch { + case health == nil: + stats.Set("ledger_health", 1, stats.T("status", "unknown")) + case *health == false: + stats.Set("ledger_health", 1, stats.T("status", "unhealthy")) + case *health == true: + stats.Set("ledger_health", 1, stats.T("status", "healthy")) } return nil }() From c0d60c941f1e992e4750d0394950e507115c38b1 Mon Sep 17 00:00:00 2001 From: Hongyu Zhou Date: Thu, 9 Mar 2023 15:45:11 -0500 Subject: [PATCH 2/4] add test cases to TestGetRowsByKeyPrefix and minor refatoring --- ldb_reader.go | 18 ++++++------ ldb_reader_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/ldb_reader.go b/ldb_reader.go index 201ffdbd..988446ac 100644 --- a/ldb_reader.go +++ b/ldb_reader.go @@ -138,7 +138,7 @@ func (reader *LDBReader) GetLedgerLatency(ctx context.Context) (time.Duration, e } } -// GetRowsByKeyPrefix returns a *Rows iterator that will supply all of the rows in +// GetRowsByKeyPrefix returns a *Rows iterator that will supply all the rows in // the family and table match the supplied primary key prefix. func (reader *LDBReader) GetRowsByKeyPrefix(ctx context.Context, familyName string, tableName string, key ...interface{}) (*Rows, error) { ctx = discardContext() @@ -201,8 +201,8 @@ func (reader *LDBReader) GetRowsByKeyPrefix(ctx context.Context, familyName stri // filling the data into the out param. // // The out param may be one of the following types: -// * pointer to struct -// * map[string]interface{} +// - pointer to struct +// - map[string]interface{} // // The key parameter can support composite keys by passing a slice type. func (reader *LDBReader) GetRowByKey( @@ -240,7 +240,7 @@ func (reader *LDBReader) GetRowByKey( // very likely use the global singleton reader, this means that we // must assume that the cache will be shared across the whole process. // The way that a PK would be changed on a table is that it would need - // to be dropped and re-created. In the mean time, this cache will + // to be dropped and re-created. In the meantime, this cache will // go stale. The way that this is dealt with is to clear the cache if // the statement encounters any execution errors. pk, err := reader.getPrimaryKey(ctx, ldbTable) // assumes RLock held @@ -367,11 +367,11 @@ func (reader *LDBReader) Ping(ctx context.Context) bool { // ensure that a supplied key is converted appropriately with respect // to the type of each PK column. func convertKeyBeforeQuery(pk schema.PrimaryKey, key []interface{}) error { + // sanity check on th length of the pk field type slice + if len(key) > len(pk.Types) { + return errors.New("insufficient key field type data") + } for i, k := range key { - // sanity check on th elength of the pk field type slice - if i >= len(pk.Types) { - return errors.New("insufficient key field type data") - } pkt := pk.Types[i] switch k := k.(type) { case string: @@ -398,7 +398,7 @@ func (reader *LDBReader) unlock() { func (reader *LDBReader) invalidatePKCache(ldbTable string) { if reader.pkCache == nil { // Cache hasn't even been initialized yet, so invalidation would - // do nothing anyways. + // do nothing anyway. return } diff --git a/ldb_reader_test.go b/ldb_reader_test.go index 612774b4..98ea9a57 100644 --- a/ldb_reader_test.go +++ b/ldb_reader_test.go @@ -160,6 +160,78 @@ func TestGetRowsByKeyPrefix(t *testing.T) { }, err: nil, }, + { + desc: "empty string key [map]", + family: "foo", + table: "multirow", + key: []interface{}{""}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "empty string keys [struct]", + family: "foo", + table: "multirow", + key: []interface{}{""}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "incomplete keys [map]", + family: "foo", + table: "multirow", + key: []interface{}{"a", ""}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "incomplete keys [struct]", + family: "foo", + table: "multirow", + key: []interface{}{"a", ""}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "out-of-ordered keys [map]", + family: "foo", + table: "multirow", + key: []interface{}{"A", "a"}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "out-of-ordered keys [struct]", + family: "foo", + table: "multirow", + key: []interface{}{"A", "a"}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "not the first key [map]", + family: "foo", + table: "multirow", + key: []interface{}{"A"}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, + { + desc: "not the first key [struct]", + family: "foo", + table: "multirow", + key: []interface{}{"A"}, + targetFunc: func() interface{} { return map[string]interface{}{} }, + expected: nil, + err: nil, + }, { desc: "no keys [map]", family: "foo", From a87f0f685b4bc4a018c348784a0e2b5374383940 Mon Sep 17 00:00:00 2001 From: Hongyu Zhou Date: Thu, 9 Mar 2023 16:08:48 -0500 Subject: [PATCH 3/4] feedback: remove ecs flag checker --- pkg/ledger/ledger_monitor.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/ledger/ledger_monitor.go b/pkg/ledger/ledger_monitor.go index 6a2e3ff3..da7d5d8b 100644 --- a/pkg/ledger/ledger_monitor.go +++ b/pkg/ledger/ledger_monitor.go @@ -80,21 +80,19 @@ func (m *Monitor) Start(ctx context.Context) { } // always instrument ledger latency even if ECS behavior is disabled. stats.Set("reflector-ledger-latency", latency) - if !m.cfg.DisableECSBehavior { - switch { - case latency <= m.cfg.MaxHealthyLatency && (health == nil || *health != true): - // set a healthy attribute - if err := m.setHealthAttribute(ctx, m.cfg.HealthyAttributeValue); err != nil { - return errors.Wrap(err, "set healthy") - } - health = pointer.ToBool(true) - case latency > m.cfg.MaxHealthyLatency && (health == nil || *health != false): - // set an unhealthy attribute - if err := m.setHealthAttribute(ctx, m.cfg.UnhealthyAttributeValue); err != nil { - return errors.Wrap(err, "set unhealthy") - } - health = pointer.ToBool(false) + switch { + case latency <= m.cfg.MaxHealthyLatency && (health == nil || *health != true): + // set a healthy attribute + if err := m.setHealthAttribute(ctx, m.cfg.HealthyAttributeValue); err != nil { + return errors.Wrap(err, "set healthy") + } + health = pointer.ToBool(true) + case latency > m.cfg.MaxHealthyLatency && (health == nil || *health != false): + // set an unhealthy attribute + if err := m.setHealthAttribute(ctx, m.cfg.UnhealthyAttributeValue); err != nil { + return errors.Wrap(err, "set unhealthy") } + health = pointer.ToBool(false) } switch { case health == nil: From 93b99e70c17b0ef7479e0643e712eb228247258c Mon Sep 17 00:00:00 2001 From: Hongyu Zhou Date: Mon, 3 Apr 2023 15:53:55 -0400 Subject: [PATCH 4/4] add sanity check for EKS to set health value --- pkg/ledger/ledger_monitor.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/ledger/ledger_monitor.go b/pkg/ledger/ledger_monitor.go index da7d5d8b..de44cc5f 100644 --- a/pkg/ledger/ledger_monitor.go +++ b/pkg/ledger/ledger_monitor.go @@ -122,6 +122,10 @@ func (m *Monitor) Start(ctx context.Context) { } func (m *Monitor) setHealthAttribute(ctx context.Context, attrValue string) error { + if m.cfg.DisableECSBehavior { + return nil + } + events.Log("Setting ECS instance attribute: %s=%s", m.cfg.AttributeName, attrValue) ecsMeta, err := m.getECSMetadata(ctx) if err != nil {