mirror of
https://github.com/cgrates/cgrates.git
synced 2026-02-15 21:29:52 +05:00
Optimize removeFilterIndexesForFilter func (#4357)
Previously made two trips (get and set) for each group of items from an index key being removed. Now, we fetch indexes once at the beginning and store the updated indexes once at the end. There was a difference compared to the previous v0.10 implementation regarding the get/set behaviour. On v0.11, we used to get only the items separately for each index and update only that index. Even though it might have been more efficient, trips to the db are still too expensive so it didn't matter much in the end.
This commit is contained in:
ionutboangiu
committed by
Dan Christian Bogos
Dan Christian Bogos
parent
5b75bacff7
commit
abba61723a
@@ -783,32 +783,32 @@ func UpdateFilterIndex(dm *DataManager, oldFlt, newFlt *Filter) (err error) {
|
||||
return
|
||||
}
|
||||
|
||||
// removeFilterIndexesForFilter removes the itemID for the index keys
|
||||
// used to remove the old indexes when a filter is updated
|
||||
// removeFilterIndexesForFilter removes itemIDs from the specified filter index keys.
|
||||
// Used to update the index map when a filter is modified.
|
||||
func removeFilterIndexesForFilter(dm *DataManager, idxItmType, tnt string,
|
||||
removeIndexKeys []string, itemIDs utils.StringSet) (err error) {
|
||||
removeIndexKeys []string, itemIDs utils.StringSet) error {
|
||||
refID := guardian.Guardian.GuardIDs(utils.EmptyString,
|
||||
config.CgrConfig().GeneralCfg().LockingTimeout, idxItmType+tnt)
|
||||
defer guardian.Guardian.UnguardIDs(refID)
|
||||
for _, idxKey := range removeIndexKeys { // delete old filters indexes for this item
|
||||
var remIndx map[string]utils.StringSet
|
||||
if remIndx, err = dm.GetIndexes(idxItmType, tnt,
|
||||
idxKey, true, false); err != nil {
|
||||
if err != utils.ErrNotFound {
|
||||
return
|
||||
}
|
||||
err = nil
|
||||
continue
|
||||
}
|
||||
for idx := range itemIDs {
|
||||
remIndx[idxKey].Remove(idx)
|
||||
}
|
||||
|
||||
if err = dm.SetIndexes(idxItmType, tnt, remIndx, true, utils.NonTransactional); err != nil {
|
||||
return
|
||||
// Retrieve current filter indexes.
|
||||
fltrIdx, err := dm.GetIndexes(idxItmType, tnt, utils.EmptyString, true, false)
|
||||
if err != nil {
|
||||
if err != utils.ErrNotFound {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Remove itemIDs from the specified index keys.
|
||||
for _, idxKey := range removeIndexKeys {
|
||||
for itemID := range itemIDs {
|
||||
fltrIdx[idxKey].Remove(itemID)
|
||||
}
|
||||
}
|
||||
return
|
||||
|
||||
// Store the updated indexes.
|
||||
return dm.SetIndexes(idxItmType, tnt, fltrIdx, true, utils.NonTransactional)
|
||||
}
|
||||
|
||||
// IsDynamicDPPath check dynamic path with ~*stats, ~*resources, ~*accounts, ~*libphonenumber, ~*asm to not be indexed
|
||||
|
||||
@@ -24,6 +24,7 @@ import (
|
||||
|
||||
"github.com/cgrates/cgrates/config"
|
||||
"github.com/cgrates/cgrates/utils"
|
||||
"github.com/google/go-cmp/cmp"
|
||||
)
|
||||
|
||||
func TestLibIndexIsDynamicDPPath(t *testing.T) {
|
||||
@@ -45,39 +46,130 @@ func TestLibIndexIsDynamicDPPath(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLibIndexRemoveFilterIndexesForFilterErrnotFound(t *testing.T) {
|
||||
func TestLibIndexRemoveFilterIndexesForFilter(t *testing.T) {
|
||||
cfg := config.NewDefaultCGRConfig()
|
||||
dataDB := NewInternalDB(nil, nil, true, cfg.DataDbCfg().Items)
|
||||
dm := NewDataManager(dataDB, cfg.CacheCfg(), nil)
|
||||
tntCtx := "cgrates.org:*sessions"
|
||||
test := struct {
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
idx map[string]utils.StringSet
|
||||
keys []string
|
||||
idx map[string]utils.StringSet // initial indexes map
|
||||
keys []string // that will be removed from the index
|
||||
itemIDs utils.StringSet
|
||||
want map[string]utils.StringSet // indexes map after remove
|
||||
}{
|
||||
name: "testing for ErrNotFound",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
{
|
||||
name: "remove one filter index from all profiles",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
keys: []string{"*string:~*req.Account:1001"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
want: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "remove one filter index from one profile",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
keys: []string{"*string:~*req.Account:1001"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP1"}),
|
||||
want: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "attempt remove non-existent filter index",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
keys: []string{"*string:~*req.Account:notfound"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
want: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "remove all filter indexes from one profile",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
keys: []string{"*string:~*req.Account:1001", "*string:~*req.Account:1002"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP1"}),
|
||||
want: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP2"}),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "remove all filter indexes from all profiles",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
"*string:~*req.Account:1002": utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
},
|
||||
keys: []string{"*string:~*req.Account:1001", "*string:~*req.Account:1002"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "remove multiple filter indexes from mixed profiles",
|
||||
idx: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2", "AP3"}),
|
||||
"*string:~*req.Destination:1010": utils.NewStringSet([]string{"AP2", "AP3"}),
|
||||
"*string:~*req.Destination:1011": utils.NewStringSet([]string{"AP1", "AP3", "AP4"}),
|
||||
"*string:~*req.Destination:1012": utils.NewStringSet([]string{"AP2"}),
|
||||
},
|
||||
keys: []string{"*string:~*req.Destination:1010", "*string:~*req.Destination:1012"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP2", "AP3"}),
|
||||
want: map[string]utils.StringSet{
|
||||
"*string:~*req.Account:1001": utils.NewStringSet([]string{"AP1", "AP2", "AP3"}),
|
||||
"*string:~*req.Destination:1011": utils.NewStringSet([]string{"AP1", "AP3", "AP4"}),
|
||||
},
|
||||
},
|
||||
keys: []string{"*string:~*req.Account:1001"},
|
||||
itemIDs: utils.NewStringSet([]string{"AP1", "AP2"}),
|
||||
}
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
t.Cleanup(func() {
|
||||
if err := dataDB.Flush(""); err != nil {
|
||||
t.Logf("failed to flush dataDB: %v", err)
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
t.Cleanup(func() {
|
||||
if err := dataDB.Flush(""); err != nil {
|
||||
t.Logf("failed to flush dataDB: %v", err)
|
||||
}
|
||||
})
|
||||
if err := dm.SetIndexes(utils.CacheAttributeFilterIndexes, tntCtx, test.idx, true, ""); err != nil {
|
||||
t.Fatalf("dm.SetFilterIndexes() returned unexpected error: %v", err)
|
||||
}
|
||||
if err := removeFilterIndexesForFilter(dm, utils.CacheAttributeFilterIndexes,
|
||||
tntCtx, test.keys, test.itemIDs); err != nil {
|
||||
t.Fatalf("removeFilterIndexesForFilter() returned unexpected error: %v", err)
|
||||
}
|
||||
got, err := dm.GetIndexes(utils.CacheAttributeFilterIndexes, tntCtx, "", true, false)
|
||||
switch len(test.want) {
|
||||
case 0:
|
||||
if !errors.Is(err, utils.ErrNotFound) {
|
||||
t.Fatalf("dm.GetFilterIndexes(%q,%q) err = %v, want %v",
|
||||
utils.CacheAttributeFilterIndexes, tntCtx, err, utils.ErrNotFound)
|
||||
}
|
||||
default:
|
||||
if err != nil {
|
||||
t.Fatalf("dm.GetFilterIndexes(%q,%q) returned unexpected error: %v",
|
||||
utils.CacheAttributeFilterIndexes, tntCtx, err)
|
||||
}
|
||||
}
|
||||
if diff := cmp.Diff(test.want, got); diff != "" {
|
||||
t.Errorf("dm.GetFilterIndexes(%q,%q) returned unexpected indexes (-want +got): \n%s",
|
||||
utils.CacheAttributeFilterIndexes, tntCtx, diff)
|
||||
}
|
||||
})
|
||||
if err := dm.SetIndexes(utils.CacheAttributeFilterIndexes, tntCtx, test.idx, true, ""); err != nil {
|
||||
t.Fatalf("dm.SetIndexes() returned unexpected error: %v", err)
|
||||
}
|
||||
err := removeFilterIndexesForFilter(dm, utils.CacheAttributeFilterIndexes, tntCtx, test.keys, test.itemIDs)
|
||||
if err != nil && !errors.Is(err, utils.ErrNotFound) {
|
||||
t.Fatalf("Expected error %v, got %v", utils.ErrNotFound, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLibIndexSplitFilterIndexErrWrongIdxKeyFormat(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user