Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] FIX loop protection #3893

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] FIX loop protection #3893

wants to merge 3 commits into from

Conversation

fgalan
Copy link
Member

@fgalan fgalan commented Jul 14, 2021

This PR improves loop protection. Until this PR, this protection only works for self-custom notifications but now it works in any case (e.g. CEP loops).

It is not easy to test this PR without adding an extra tool (i.e. accumulator-py like) to redirect a notification back to the CB with a modification in the attribute name (pure self-notification doesn't work, as the attribute value doesn't change so a new notification is not triggered). Thus in this case no ftest can be added and we have tested manually the following way:

  1. Hack the CB code so attributes with same value also trigger notifications:
diff --git a/src/lib/mongoBackend/MongoCommonUpdate.cpp b/src/lib/mongoBackend/MongoCommonUpdate.cpp
index a3fd978f0..47bb19e3a 100644
--- a/src/lib/mongoBackend/MongoCommonUpdate.cpp
+++ b/src/lib/mongoBackend/MongoCommonUpdate.cpp
@@ -306,6 +306,8 @@ static bool equalMetadata(const orion::BSONObj& md1, const orion::BSONObj& md2)
 */
 static bool attrValueChanges(const orion::BSONObj& attr, ContextAttribute* caP, const bool& forcedUpdate, ApiVersion apiVersion)
 {
+  return true;
+
   /* Not finding the attribute field at MongoDB is considered as an implicit "" */
   if (!attr.hasField(ENT_ATTRS_VALUE))
   {
  1. Check that the following tets works (based in exiting cases/2937_self_notification_protection/self_notification_one_hop.test)
--NAME--
Self notification protection one-hop case with regular (non custom) subscription

--SHELL-INIT--
dbInit CB
brokerStart CB

--SHELL--

#
# 01. Create self-subscription for entity E attribute A
# 02. Create entity with A = boom
# 03. Wait for a while and check that only one notification was sent
# 04. Get entity with value boomx
# 05. Check the warning about loop detection in the log

echo "01. Create self-subscription for entity E attribute A"
echo "====================================================="
payload='{
  "subject": {
    "entities": [
      {
        "id" : "E"
      }
    ],
    "condition": {
      "attrs": [ "A" ]
    }
  },
  "notification": {
    "http": {
      "url": "http://localhost:'${CB_PORT}'/v2/op/notify"
    }
  }
}'
orionCurl --url /v2/subscriptions --payload "$payload"
echo
echo


echo "02. Create entity with A = boom"
echo "==============================="
payload='{
  "id": "E",
  "type": "Thing",
  "A": {
    "value": "boom",
    "type": "Text"
  }
}'
orionCurl --url /v2/entities --payload "$payload"
echo
echo


echo "03. Wait for a while and check that only one notification was sent"
echo "=================================================================="
sleep 5s
orionCurl --url /v2/subscriptions
echo
echo


echo "04. Get entity with value boom"
echo "=============================="
orionCurl --url /v2/entities/E
echo
echo


echo "05. Check the warning about loop detection in the log"
echo "====================================================="
egrep 'WARN' /tmp/contextBroker.log
echo
echo


--REGEXPECT--
01. Create self-subscription for entity E attribute A
=====================================================
HTTP/1.1 201 Created
Content-Length: 0
Location: /v2/subscriptions/REGEX([0-9a-f]{24})
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)



02. Create entity with A = boom
===============================
HTTP/1.1 201 Created
Content-Length: 0
Location: /v2/entities/E?type=Thing
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)



03. Wait for a while and check that only one notification was sent
==================================================================
HTTP/1.1 200 OK
Content-Length: 372
Content-Type: application/json
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)

[
    {
        "id": "REGEX([0-9a-f]{24})",
        "notification": {
            "attrs": [],
            "attrsFormat": "normalized",
            "http": {
                "url": "http://localhost:REGEX(\d+)/v2/op/notify"
            },
            "lastNotification": "REGEX(.*)",
            "lastSuccess": "REGEX(.*)",
            "lastSuccessCode": 200,
            "onlyChangedAttrs": false,
            "timesSent": 1
        },
        "status": "active",
        "subject": {
            "condition": {
                "attrs": [
                    "A"
                ]
            },
            "entities": [
                {
                    "id": "E"
                }
            ]
        }
    }
]


04. Get entity with value boom
==============================
HTTP/1.1 200 OK
Content-Length: 74
Content-Type: application/json
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)

{
    "A": {
        "metadata": {},
        "type": "Text",
        "value": "boom"
    },
    "id": "E",
    "type": "Thing"
}


05. Check the warning about loop detection in the log
=====================================================
REGEX(.*) msg=Notification loop detected for entity id <E> type <Thing>, skipping subscription triggering


--TEARDOWN--
brokerStop CB
dbDrop CB
  1. If we revert the change done in this PR and run the tests again, we can check it fails ("timesSent": 961 instead of "timesSent": 1 and step 05 doesn't show the expected line in the logs). This validates the fix done.

@fgalan fgalan changed the title FIX loop protectiong FIX loop protection Jul 14, 2021
@fgalan fgalan changed the title FIX loop protection [WIP] FIX loop protection Jul 14, 2021
@fgalan
Copy link
Member Author

fgalan commented Jul 14, 2021

Fail in 3694_logs_improvements/log_notifications.test case shows that the fix is not so simple as I originally thought :)

Removing the (ngsiV2AttrsFormat == "custom") causes problems in POST /v2/op/update when the same entity is included more than one time in the batch, as CB processes each entity in the batch independently.

Thus, if for instance we have POST /v2/op/update - CorrA – {E1, E1}, when first E1 is processed CB stores in DB E1_last_correlator=CoorA. When processing second E1 the correlator in the request (CoorA) is the same than E1_last_correlator in DB (CoorA) thus wrongly detecting a loop. The log_notifications.test test is failing due to this.

A possible solution is not storing any last_correlator in DB while the batch is being processed. Instead of that, store in temporal memory structure, then flush that memory structure to DB once batch processing ends. However, this solution may be complex.

@fgalan
Copy link
Member Author

fgalan commented Jun 11, 2024

A possible solution is not storing any last_correlator in DB while the batch is being processed. Instead of that, store in temporal memory structure, then flush that memory structure to DB once batch processing ends. However, this solution may be complex.

This would be easier if issue #1153 is implemented. Do that issue before completing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant