@cryptotaxi247 / netdata-1 / commits / 0542661ff

Improvements for labels handling (#16172)

* Add additional checks rrdlabels_find_label_with_key_unsafe finds a label with specified key (not only if it exists but with difefrent value) Quick check if label already exists (avoids JudyLIns) * Add migration unit test Add additional unit test to verify that adding a label with the same key will replace its value

Stelios Fragkakis committed Oct 18, 2023 at 15:45 UTC 0542661fff8527ad8d8682a7fa71a24b1878549e

1 file changed +127 -24

database/rrdlabels.c

+127 -24

@@ -671,6 +671,7 @@ void rrdlabels_destroy(RRDLABELS *labels)

671 freez(labels);

672 }

673

674 +// Check in labels to see if we have the key specified in label

675 static RRDLABEL *rrdlabels_find_label_with_key_unsafe(RRDLABELS *labels, RRDLABEL *label)

676 {

677 if (unlikely(!labels))

@@ -682,7 +683,7 @@ static RRDLABEL *rrdlabels_find_label_with_key_unsafe(RRDLABELS *labels, RRDLABE

683 RRDLABEL *found = NULL;

684 while ((PValue = JudyLFirstThenNext(labels->JudyL, &Index, &first_then_next))) {

685 RRDLABEL *lb = (RRDLABEL *)Index;

685 - if (lb->index.key == label->index.key && lb->index.value != label->index.value) {

686 + if (lb->index.key == label->index.key) {

687 found = (RRDLABEL *)Index;

688 break;

689 }

@@ -695,39 +696,42 @@ static RRDLABEL *rrdlabels_find_label_with_key_unsafe(RRDLABELS *labels, RRDLABE

696

697 static void labels_add_already_sanitized(RRDLABELS *labels, const char *key, const char *value, RRDLABEL_SRC ls)

698 {

698 - RRDLABEL *label = add_label_name_value(key, value);

699 + RRDLABEL *new_label = add_label_name_value(key, value);

700

701 spinlock_lock(&labels->spinlock);

702

702 - RRDLABEL *old_key = rrdlabels_find_label_with_key_unsafe(labels, label);

703 + RRDLABEL *old_label_with_key = rrdlabels_find_label_with_key_unsafe(labels, new_label);

704 +

705 + if (old_label_with_key == new_label) {

706 + spinlock_unlock(&labels->spinlock);

707 + delete_label(new_label);

708 + return;

709 + }

710

711 size_t mem_before_judyl = JudyLMemUsed(labels->JudyL);

712

706 - Pvoid_t *PValue = JudyLIns(&labels->JudyL, (Word_t) label, PJE0);

707 - if(unlikely(!PValue || PValue == PJERR))

713 + Pvoid_t *PValue = JudyLIns(&labels->JudyL, (Word_t)new_label, PJE0);

714 + if (!PValue || PValue == PJERR)

715 fatal("RRDLABELS: corrupted labels JudyL array");

716

710 - if (!*PValue) {

711 - RRDLABEL_SRC new_ls;

712 - if (old_key)

713 - new_ls = ((ls & ~(RRDLABEL_FLAG_NEW | RRDLABEL_FLAG_OLD)) | RRDLABEL_FLAG_OLD);

714 - else

715 - new_ls = ((ls & ~(RRDLABEL_FLAG_NEW | RRDLABEL_FLAG_OLD)) | RRDLABEL_FLAG_NEW);

716 - *((RRDLABEL_SRC *)PValue) = new_ls;

717 + RRDLABEL_SRC new_ls = (ls & ~(RRDLABEL_FLAG_NEW | RRDLABEL_FLAG_OLD));

718 + labels->version++;

719

718 - labels->version++;

720 + if (old_label_with_key) {

721 + (void)JudyLDel(&labels->JudyL, (Word_t)old_label_with_key, PJE0);

722 + new_ls |= RRDLABEL_FLAG_OLD;

723 + } else

724 + new_ls |= RRDLABEL_FLAG_NEW;

725

720 - if (old_key) {

721 - (void)JudyLDel(&labels->JudyL, (Word_t) old_key, PJE0);

722 - delete_label((RRDLABEL *)old_key);

723 - }

724 - size_t mem_after_judyl = JudyLMemUsed(labels->JudyL);

725 - STATS_PLUS_MEMORY(&dictionary_stats_category_rrdlabels, 0, mem_after_judyl - mem_before_judyl, 0);

726 - }

727 - else

728 - delete_label(label);

726 + *((RRDLABEL_SRC *)PValue) = new_ls;

727 +

728 + size_t mem_after_judyl = JudyLMemUsed(labels->JudyL);

729 + STATS_PLUS_MEMORY(&dictionary_stats_category_rrdlabels, 0, mem_after_judyl - mem_before_judyl, 0);

730

731 spinlock_unlock(&labels->spinlock);

732 +

733 + if (old_label_with_key)

734 + delete_label((RRDLABEL *)old_label_with_key);

735 }

736

737 void rrdlabels_add(RRDLABELS *labels, const char *name, const char *value, RRDLABEL_SRC ls)

@@ -984,7 +988,7 @@ int rrdlabels_walkthrough_read(RRDLABELS *labels, int (*callback)(const char *na

988 // migrate an existing label list to a new list

989

990 void rrdlabels_migrate_to_these(RRDLABELS *dst, RRDLABELS *src) {

987 - if (!dst || !src)

991 + if (!dst || !src || (dst == src))

992 return;

993

994 spinlock_lock(&dst->spinlock);

@@ -1025,7 +1029,7 @@ void rrdlabels_migrate_to_these(RRDLABELS *dst, RRDLABELS *src) {

1029

1030 void rrdlabels_copy(RRDLABELS *dst, RRDLABELS *src)

1031 {

1028 - if (!dst || !src)

1032 + if (!dst || !src || (dst == src))

1033 return;

1034

1035 RRDLABEL *label;

@@ -1265,6 +1269,9 @@ void rrdlabels_to_buffer_json_members(RRDLABELS *labels, BUFFER *wb)

1269

1270 size_t rrdlabels_entries(RRDLABELS *labels __maybe_unused)

1271 {

1272 + if (unlikely(!labels))

1273 + return 0;

1274 +

1275 size_t count;

1276 spinlock_lock(&labels->spinlock);

1277 count = JudyLCount(labels->JudyL, 0, -1, PJE0);

@@ -1274,6 +1281,9 @@ size_t rrdlabels_entries(RRDLABELS *labels __maybe_unused)

1281

1282 size_t rrdlabels_version(RRDLABELS *labels __maybe_unused)

1283 {

1284 + if (unlikely(!labels))

1285 + return 0;

1286 +

1287 return (size_t) labels->version;

1288 }

1289

@@ -1405,6 +1415,97 @@ int rrdlabels_unittest_add_pairs() {

1415 return errors;

1416 }

1417

1418 +int rrdlabels_unittest_double_check() {

1419 + fprintf(stderr, "\n%s() tests\n", __FUNCTION__);

1420 +

1421 + int errors = 1;

1422 + int ret = 0;

1423 + RRDLABELS *labels = rrdlabels_create();

1424 +

1425 + const char *pair = "key1=value1";

1426 +

1427 + struct rrdlabels_unittest_add_a_pair tmp = {

1428 + .pair = pair,

1429 + .expected_name = "key1",

1430 + .expected_value = NULL,

1431 + .errors = 0

1432 + };

1433 +

1434 + fprintf(stderr, "rrdlabels_add_pair(labels, %s) ...\n ", pair);

1435 +

1436 + rrdlabels_add_pair(labels, pair, RRDLABEL_SRC_CONFIG);

1437 + size_t count = rrdlabels_entries(labels);

1438 + fprintf(stderr, "Added one key with \"value1\", entries found %zu\n", count);

1439 + tmp.expected_value = "value1";

1440 + ret = rrdlabels_walkthrough_read(labels, rrdlabels_unittest_add_a_pair_callback, &tmp);

1441 +

1442 + fprintf(stderr, "Adding key with same value \"value1\" (collision check)\n");

1443 + rrdlabels_add_pair(labels, pair, RRDLABEL_SRC_CONFIG);

1444 + count = rrdlabels_entries(labels);

1445 + fprintf(stderr, "Added same key again \"value1\", entries found %zu\n", count);

1446 +

1447 + ret = rrdlabels_walkthrough_read(labels, rrdlabels_unittest_add_a_pair_callback, &tmp);

1448 +

1449 + // Add same key with different value

1450 + pair = "key1=value2";

1451 + rrdlabels_add_pair(labels, pair, RRDLABEL_SRC_CONFIG);

1452 + count = rrdlabels_entries(labels);

1453 + fprintf(stderr, "Added same key again with \"value2\", entries found %zu\n", count);

1454 +

1455 + tmp.expected_value = "value2";

1456 + ret = rrdlabels_walkthrough_read(labels, rrdlabels_unittest_add_a_pair_callback, &tmp);

1457 +

1458 + fprintf(stderr, "Adding key with same value \"value2\" (collision check)\n");

1459 + rrdlabels_add_pair(labels, pair, RRDLABEL_SRC_CONFIG);

1460 + count = rrdlabels_entries(labels);

1461 + fprintf(stderr, "Added same key again with \"value2\", entries found %zu\n", count);

1462 +

1463 + ret = rrdlabels_walkthrough_read(labels, rrdlabels_unittest_add_a_pair_callback, &tmp);

1464 + errors = tmp.errors;

1465 + if(ret != 1) {

1466 + fprintf(stderr, "failed to get \"%s\" label", "key1");

1467 + errors++;

1468 + }

1469 +

1470 + if(!errors)

1471 + fprintf(stderr, " OK, name='%s' and value='%s'\n", tmp.name, tmp.value?tmp.value:"(null)");

1472 + else

1473 + fprintf(stderr, " FAILED\n");

1474 +

1475 + rrdlabels_destroy(labels);

1476 +

1477 + return errors;

1478 +}

1479 +

1480 +int rrdlabels_unittest_migrate_check() {

1481 + fprintf(stderr, "\n%s() tests\n", __FUNCTION__);

1482 +

1483 + RRDLABELS *labels1 = NULL;

1484 + RRDLABELS *labels2 = NULL;

1485 +

1486 + labels1 = rrdlabels_create();

1487 + labels2 = rrdlabels_create();

1488 +

1489 + rrdlabels_add(labels1, "key1", "value1", RRDLABEL_SRC_CONFIG);

1490 + rrdlabels_add(labels1, "key1", "value2", RRDLABEL_SRC_CONFIG);

1491 +

1492 + rrdlabels_add(labels2, "new_key1", "value2", RRDLABEL_SRC_CONFIG);

1493 + rrdlabels_add(labels2, "new_key2", "value2", RRDLABEL_SRC_CONFIG);

1494 + rrdlabels_add(labels2, "key1", "value2", RRDLABEL_SRC_CONFIG);

1495 +

1496 + fprintf(stderr, "Labels1 entries found %zu (should be 1)\n", rrdlabels_entries(labels1));

1497 + fprintf(stderr, "Labels2 entries found %zu (should be 3)\n", rrdlabels_entries(labels2));

1498 +

1499 + rrdlabels_migrate_to_these(labels1, labels2);

1500 + fprintf(stderr, "labels1 (migrated) entries found %zu (should be 3)\n", rrdlabels_entries(labels1));

1501 + size_t entries = rrdlabels_entries(labels1);

1502 +

1503 + rrdlabels_destroy(labels1);

1504 + rrdlabels_destroy(labels2);

1505 +

1506 + return entries != 3;

1507 +}

1508 +

1509 int rrdlabels_unittest_check_simple_pattern(RRDLABELS *labels, const char *pattern, bool expected) {

1510 fprintf(stderr, "rrdlabels_match_simple_pattern(labels, \"%s\") ... ", pattern);

1511

@@ -1497,6 +1598,8 @@ int rrdlabels_unittest(void) {

1598 errors += rrdlabels_unittest_sanitization();

1599 errors += rrdlabels_unittest_add_pairs();

1600 errors += rrdlabels_unittest_simple_pattern();

1601 + errors += rrdlabels_unittest_double_check();

1602 + errors += rrdlabels_unittest_migrate_check();

1603

1604 fprintf(stderr, "%d errors found\n", errors);

1605 return errors;