Subscripton status "failed" is a crafted (not real in DB/cache) status

[fgalan]

Currently we have this in Subscrpition::toJson() rendering logic:

  if ((this->notification.lastFailure > 0) && (this->notification.lastFailure > this->notification.lastSuccess))
  {
    jh.addString("status", "failed");
  }
  else
  {
    jh.addString("status", this->status);
  }

Thus, failed is not a real status coming from csubs document/cache.

This is a bit weird, but I'm not sure which should be do. Some alternatives:

1. Store failed in DB/cache as any other status (active, inactive, etc.). That would simplify the rendering logic in Subscription::toJson() but involve dealing with seting failed in the proper place (in the code dealing with notification fails). Another more difficult to solve problem of "conceptual" kind is how to deal with some status transition situation, e.g. a subscription that is in failed status can be changed to active taking into account we don't have information about if the situation that make it to fail has disappeared?
2. Considering failed/ok condition as orthogonal to active/inactive condition, it could be an extra field, e.g. "failed": true|false. Thus, "status": "active", "failed": true will clearly identify that a subscription is active (i.e. updates will trigger notifications) and, at the same time, last notification caused a fail.
3. Remove status failed at all. The user may infer if the subscription is failling or not based in lastNotification/lastFailure/lastSuccess information, basically doing himself the same checking we currently have in the if-clause above.

Feedback welcome! :)

[fgalan]

Moreover, the this->notification.lastFailure > this->notification.lastSuccess condition is weak as two notifications coud be sent in the same second (first one successfu, second one failing) and the status will end in "active" instead of "failed"

[mapedraza]

IMHO failed/ok condition is totally orthogonal to active/inactive condition. For example, if notification pass to inactive after several times trying to send the notification as proposed in #3962, It is needed to know about the status of the subscription (inactive) and also about the status of the notification (failed), to identify the real situation.

If the condition this->notification.lastFailure > this->notification.lastSuccess is going to be always valid for a failing notification, maybe adding millisecond support to this parameters could be sufficient to solve this situation, and for sure is easier to maintain and implement.

[fgalan]

If the condition this->notification.lastFailure > this->notification.lastSuccess is going to be always valid for a failing notification, maybe adding millisecond support to this parameters could be sufficient to solve this situation, and for sure is easier to maintain and implement.

Using milliseconds for lastNotification/lastFailure/lastSuccess has been considered in issue #3671

[fgalan]

IMHO failed/ok condition is totally orthogonal to active/inactive condition. For example, if notification pass to inactive after several times trying to send the notification as proposed in #3962, It is needed to know about the status of the subscription (inactive) and also about the status of the notification (failed), to identify the real situation.

So +1 to option 2 I guess :)

[fgalan]

Note that failed is one of the status defined in the NGSIv2 specification (http://telefonicaid.github.io/fiware-orion/api/v2/stable/)

So if we go for option 2 or 3 we have to take this into consideration (eg. documenting this deviation from the spec)

[fgalan]

The description of status in doc/manuals/admin/database_model.md could need to be fixed depending on how this issue gets solved:

• status: either active (for active subscriptions), inactive (for inactive subscriptions) or
  oneshot (for oneshot subscriptions). Note that NGSIv2 API consider additional states (e.g. failed or expired)
  but they never hit the DB (they are managed by Orion).

[fgalan]

At the end we are going to implement option 3:

Remove status failed at all. The user may infer if the subscription is failling or not based in lastNotification/lastFailure/lastSuccess information, basically doing himself the same checking we currently have in the if-clause above.

At the same can be achieved just checking new field failsCounter is greater than 0

[fgalan]

PR #4006