[asterisk-commits] res pjsip: Fix statsd regression. (asterisk[master])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Jul 13 07:41:48 CDT 2016
Joshua Colp has submitted this change and it was merged.
Change subject: res_pjsip: Fix statsd regression.
......................................................................
res_pjsip: Fix statsd regression.
The ASTERISK-25904 change-id I8fad8aae9305481469c38d2146e1ba3a56d3108f
patch introduced several regressions when the newly created "Updated"
state goes out for each endpoint registration refresh.
1) It restarted any OPTIONS RTT ping cycle.
2) It would interfere with a currently active ping and throw off that
ping's resulting RTT calculation.
3) It cleared the RTT time each time the endpoint was refreshed.
4) The cleared RTT time was sent out as a statsd update each time.
5) It created two AMI events for each update.
* Revert the original patch and reimplement it. Now the current contact
status state is re-sent instead of the state being momentarily toggled
every time the endpoint refreshes its registration. The statsd events are
not created for the re-sent refresh because they are sent after every
OPTIONS ping.
ASTERISK-26160 #close
Reported by: Matt Jordan
Change-Id: Ie072be790fbb2a8f5c1c874266e4143fa31f66d1
---
M CHANGES
M include/asterisk/res_pjsip.h
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip/pjsip_options.c
4 files changed, 103 insertions(+), 46 deletions(-)
Approvals:
Mark Michelson: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Verified
diff --git a/CHANGES b/CHANGES
index 6e6bec6..cbe03fd 100644
--- a/CHANGES
+++ b/CHANGES
@@ -397,8 +397,6 @@
"contact_deny" - List of Contact header addresses to deny
"contact_permit" - List of Contact header addresses to permit
- * Added new status Updated to AMI event ContactStatus on update registration
-
* Added "reg_server" to contacts.
If the Asterisk system name is set in asterisk.conf, it will be stored
into the "reg_server" field in the ps_contacts table to facilitate
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index f9f9e20..f43a11c 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -271,7 +271,6 @@
UNKNOWN,
CREATED,
REMOVED,
- UPDATED,
};
/*!
@@ -296,6 +295,8 @@
int64_t rtt;
/*! Last status for a contact (default - unavailable) */
enum ast_sip_contact_status_type last_status;
+ /*! TRUE if the contact was refreshed. e.g., re-registered */
+ unsigned int refresh:1;
};
/*!
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 8a5ff41..6890ced 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -105,6 +105,40 @@
ast_devstate_changed(AST_DEVICE_UNKNOWN, AST_DEVSTATE_CACHABLE, "PJSIP/%s", ast_endpoint_get_resource(endpoint));
}
+static void endpoint_publish_contact_status(struct ast_endpoint *endpoint, struct ast_sip_contact_status *contact)
+{
+ struct ast_json *blob;
+ char rtt[32];
+
+ snprintf(rtt, sizeof(rtt), "%" PRId64, contact->rtt);
+ blob = ast_json_pack("{s: s, s: s, s: s, s: s, s: s}",
+ "contact_status", ast_sip_get_contact_status_label(contact->status),
+ "aor", contact->aor,
+ "uri", contact->uri,
+ "roundtrip_usec", rtt,
+ "endpoint_name", ast_endpoint_get_resource(endpoint));
+ if (blob) {
+ ast_endpoint_blob_publish(endpoint, ast_endpoint_contact_state_type(), blob);
+ ast_json_unref(blob);
+ }
+}
+
+/*! \brief Callback function for publishing the status of an endpoint */
+static int persistent_endpoint_publish_status(void *obj, void *arg, int flags)
+{
+ struct sip_persistent_endpoint *persistent = obj;
+ struct ast_endpoint *endpoint = persistent->endpoint;
+ struct ast_sip_contact_status *status = arg;
+
+ /* If the status' aor isn't one of the endpoint's, we skip */
+ if (!strstr(persistent->aors, status->aor)) {
+ return 0;
+ }
+
+ endpoint_publish_contact_status(endpoint, status);
+ return 0;
+}
+
/*! \brief Callback function for changing the state of an endpoint */
static int persistent_endpoint_update_state(void *obj, void *arg, int flags)
{
@@ -112,29 +146,16 @@
struct ast_endpoint *endpoint = persistent->endpoint;
struct ast_sip_contact_status *status = arg;
struct ao2_container *contacts;
- struct ast_json *blob;
struct ao2_iterator i;
struct ast_sip_contact *contact;
enum ast_endpoint_state state = AST_ENDPOINT_OFFLINE;
- if (status) {
- char rtt[32];
-
- /* If the status' aor isn't one of the endpoint's, we skip */
- if (!strstr(persistent->aors, status->aor)) {
- return 0;
- }
-
- snprintf(rtt, sizeof(rtt), "%" PRId64, status->rtt);
- blob = ast_json_pack("{s: s, s: s, s: s, s: s, s: s}",
- "contact_status", ast_sip_get_contact_status_label(status->status),
- "aor", status->aor,
- "uri", status->uri,
- "roundtrip_usec", rtt,
- "endpoint_name", ast_endpoint_get_resource(endpoint));
- ast_endpoint_blob_publish(endpoint, ast_endpoint_contact_state_type(), blob);
- ast_json_unref(blob);
+ /* If the status' aor isn't one of the endpoint's, we skip */
+ if (!strstr(persistent->aors, status->aor)) {
+ return 0;
}
+
+ endpoint_publish_contact_status(endpoint, status);
/* Find all the contacts for this endpoint. If ANY are available,
* mark the endpoint as ONLINE.
@@ -222,6 +243,13 @@
{
struct ast_sip_contact_status *contact_status = (struct ast_sip_contact_status *)object;
+ if (contact_status->refresh) {
+ /* We are only re-publishing the contact status. */
+ ao2_callback(persistent_endpoints, OBJ_NODATA,
+ persistent_endpoint_publish_status, contact_status);
+ return;
+ }
+
/* If rtt_start is set (this is the outgoing OPTIONS), ignore. */
if (contact_status->rtt_start.tv_sec > 0) {
return;
diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index dc3bce0..808ee17 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -43,7 +43,6 @@
[UNKNOWN] = "Unknown",
[CREATED] = "Created",
[REMOVED] = "Removed",
- [UPDATED] = "Updated",
};
static const char *short_status_map [] = {
@@ -52,7 +51,6 @@
[UNKNOWN] = "Unknown",
[CREATED] = "Created",
[REMOVED] = "Removed",
- [UPDATED] = "Updated",
};
const char *ast_sip_get_contact_status_label(const enum ast_sip_contact_status_type status)
@@ -157,7 +155,7 @@
* \brief Update an ast_sip_contact_status's elements.
*/
static void update_contact_status(const struct ast_sip_contact *contact,
- enum ast_sip_contact_status_type value)
+ enum ast_sip_contact_status_type value, int is_contact_refresh)
{
RAII_VAR(struct ast_sip_contact_status *, status, NULL, ao2_cleanup);
RAII_VAR(struct ast_sip_contact_status *, update, NULL, ao2_cleanup);
@@ -169,6 +167,26 @@
return;
}
+ if (is_contact_refresh
+ && status->status == CREATED) {
+ /*
+ * The contact status hasn't been updated since creation
+ * and we don't want to re-send a created status.
+ */
+ if (contact->qualify_frequency
+ || status->rtt_start.tv_sec > 0) {
+ /* Ignore, the status will change soon. */
+ return;
+ }
+
+ /*
+ * Convert to a regular contact status update
+ * because the status may never change.
+ */
+ is_contact_refresh = 0;
+ value = UNKNOWN;
+ }
+
update = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
ast_sorcery_object_get_id(status));
if (!update) {
@@ -178,22 +196,35 @@
}
ast_string_field_set(update, uri, contact->uri);
- update->last_status = status->status;
- update->status = value;
- /* if the contact is available calculate the rtt as
- the diff between the last start time and "now" */
- update->rtt = update->status == AVAILABLE && status->rtt_start.tv_sec > 0 ?
- ast_tvdiff_us(ast_tvnow(), status->rtt_start) : 0;
- update->rtt_start = ast_tv(0, 0);
+ if (is_contact_refresh) {
+ /* Copy everything just to set the refresh flag. */
+ update->status = status->status;
+ update->last_status = status->last_status;
+ update->rtt = status->rtt;
+ update->rtt_start = status->rtt_start;
+ update->refresh = 1;
+ } else {
+ update->last_status = status->status;
+ update->status = value;
- ast_test_suite_event_notify("AOR_CONTACT_QUALIFY_RESULT",
- "Contact: %s\r\n"
- "Status: %s\r\n"
- "RTT: %" PRId64,
- ast_sorcery_object_get_id(update),
- ast_sip_get_contact_status_label(update->status),
- update->rtt);
+ /*
+ * if the contact is available calculate the rtt as
+ * the diff between the last start time and "now"
+ */
+ update->rtt = update->status == AVAILABLE && status->rtt_start.tv_sec > 0
+ ? ast_tvdiff_us(ast_tvnow(), status->rtt_start)
+ : 0;
+ update->rtt_start = ast_tv(0, 0);
+
+ ast_test_suite_event_notify("AOR_CONTACT_QUALIFY_RESULT",
+ "Contact: %s\r\n"
+ "Status: %s\r\n"
+ "RTT: %" PRId64,
+ ast_sorcery_object_get_id(update),
+ ast_sip_get_contact_status_label(update->status),
+ update->rtt);
+ }
if (ast_sorcery_update(ast_sip_get_sorcery(), update)) {
ast_log(LOG_ERROR, "Unable to update ast_sip_contact_status for contact %s\n",
@@ -306,10 +337,10 @@
/* Fall through */
case PJSIP_EVENT_TRANSPORT_ERROR:
case PJSIP_EVENT_TIMER:
- update_contact_status(contact, UNAVAILABLE);
+ update_contact_status(contact, UNAVAILABLE, 0);
break;
case PJSIP_EVENT_RX_MSG:
- update_contact_status(contact, AVAILABLE);
+ update_contact_status(contact, AVAILABLE, 0);
break;
}
ao2_cleanup(contact);
@@ -365,7 +396,7 @@
!= PJ_SUCCESS) {
ast_log(LOG_ERROR, "Unable to send request to qualify contact %s\n",
contact->uri);
- update_contact_status(contact, UNAVAILABLE);
+ update_contact_status(contact, UNAVAILABLE, 0);
ao2_ref(contact, -1);
return -1;
}
@@ -525,7 +556,7 @@
schedule_qualify(contact, contact->qualify_frequency * 1000);
} else {
- update_contact_status(contact, UNKNOWN);
+ update_contact_status(contact, UNKNOWN, 0);
}
}
@@ -544,8 +575,7 @@
*/
static void contact_updated(const void *obj)
{
- update_contact_status((struct ast_sip_contact *) obj, UPDATED);
- qualify_and_schedule((struct ast_sip_contact *) obj);
+ update_contact_status(obj, AVAILABLE, 1);
}
/*!
@@ -574,8 +604,8 @@
static const struct ast_sorcery_observer contact_observer = {
.created = contact_created,
+ .updated = contact_updated,
.deleted = contact_deleted,
- .updated = contact_updated
};
static pj_bool_t options_start(void)
@@ -1051,7 +1081,7 @@
if (contact->qualify_frequency) {
schedule_qualify(contact, initial_interval);
} else {
- update_contact_status(contact, UNKNOWN);
+ update_contact_status(contact, UNKNOWN, 0);
}
}
--
To view, visit https://gerrit.asterisk.org/3176
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie072be790fbb2a8f5c1c874266e4143fa31f66d1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-commits
mailing list