Fix label copy to correctly handle duplicate keys (#16249)
Fix rrdlabel copy to correctly handle duplicate keys Enhance the corresponding unit tests
Stelios Fragkakis committed
Oct 20, 2023 at 17:28 UTC
8f17bbc1594da3a6516ac93ce5750c495ae2b29e
1 file changed
+49
-6
database/rrdlabels.c
+49
-6
@@ -1038,23 +1038,34 @@ void rrdlabels_copy(RRDLABELS *dst, RRDLABELS *src)
1038
spinlock_lock(&dst->spinlock);
1039
spinlock_lock(&src->spinlock);
1040
1041
+ size_t mem_before_judyl = JudyLMemUsed(dst->JudyL);
1042
+ bool update_statistics = false;
1043
lfe_start_nolock(src, label, ls)
1044
{
1043
- size_t mem_before_judyl = JudyLMemUsed(dst->JudyL);
1045
+ RRDLABEL *old_label_with_key = rrdlabels_find_label_with_key_unsafe(dst, label);
1046
+ if (old_label_with_key && old_label_with_key == label)
1047
+ continue;
1048
+
1049
Pvoid_t *PValue = JudyLIns(&dst->JudyL, (Word_t)label, PJE0);
1050
if(unlikely(!PValue || PValue == PJERR))
1051
fatal("RRDLABELS: corrupted labels array");
1052
1053
if (!*PValue) {
1054
dup_label(label);
1050
- size_t mem_after_judyl = JudyLMemUsed(dst->JudyL);
1051
- STATS_PLUS_MEMORY(&dictionary_stats_category_rrdlabels, 0, mem_after_judyl - mem_before_judyl, 0);
1055
*((RRDLABEL_SRC *)PValue) = ls;
1056
+ dst->version++;
1057
+ update_statistics = true;
1058
+ if (old_label_with_key) {
1059
+ (void)JudyLDel(&dst->JudyL, (Word_t)old_label_with_key, PJE0);
1060
+ delete_label((RRDLABEL *)old_label_with_key);
1061
+ }
1062
}
1063
}
1064
lfe_done_nolock();
1056
-
1057
- dst->version = src->version;
1065
+ if (update_statistics) {
1066
+ size_t mem_after_judyl = JudyLMemUsed(dst->JudyL);
1067
+ STATS_PLUS_MEMORY(&dictionary_stats_category_rrdlabels, 0, mem_after_judyl - mem_before_judyl, 0);
1068
+ }
1069
1070
spinlock_unlock(&src->spinlock);
1071
spinlock_unlock(&dst->spinlock);
@@ -1503,7 +1514,39 @@ int rrdlabels_unittest_migrate_check() {
1514
rrdlabels_destroy(labels1);
1515
rrdlabels_destroy(labels2);
1516
1506
- return entries != 3;
1517
+ if (entries != 3)
1518
+ return 1;
1519
+
1520
+ // Copy test
1521
+ labels1 = rrdlabels_create();
1522
+ labels2 = rrdlabels_create();
1523
+
1524
+ rrdlabels_add(labels1, "key1", "value1", RRDLABEL_SRC_CONFIG);
1525
+ rrdlabels_add(labels1, "key2", "value1", RRDLABEL_SRC_CONFIG);
1526
+ rrdlabels_add(labels1, "key3", "value1", RRDLABEL_SRC_CONFIG);
1527
+ rrdlabels_add(labels1, "key4", "value1", RRDLABEL_SRC_CONFIG); // 4 keys
1528
+
1529
+ rrdlabels_add(labels2, "key1", "value10", RRDLABEL_SRC_CONFIG);
1530
+ rrdlabels_add(labels2, "key2", "value1", RRDLABEL_SRC_CONFIG);
1531
+ rrdlabels_add(labels2, "key0", "value1", RRDLABEL_SRC_CONFIG);
1532
+
1533
+ rrdlabels_copy(labels1, labels2); // labels1 should have 5 keys
1534
+
1535
+ entries = rrdlabels_entries(labels1);
1536
+ fprintf(stderr, "labels1 (copied) entries found %zu (should be 5)\n", rrdlabels_entries(labels1));
1537
+ if (entries != 5)
1538
+ return 1;
1539
+
1540
+ rrdlabels_add(labels1, "key100", "value1", RRDLABEL_SRC_CONFIG);
1541
+ rrdlabels_copy(labels2, labels1); // labels2 should have 6 keys
1542
+ entries = rrdlabels_entries(labels1);
1543
+
1544
+ fprintf(stderr, "labels2 (copied) entries found %zu (should be 6)\n", rrdlabels_entries(labels1));
1545
+
1546
+ rrdlabels_destroy(labels1);
1547
+ rrdlabels_destroy(labels2);
1548
+
1549
+ return entries != 6;
1550
}
1551
1552
int rrdlabels_unittest_check_simple_pattern(RRDLABELS *labels, const char *pattern, bool expected) {