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

feat!: Ensure that all autoevent resource scheduler gets invoked properly during every boot of the target machine #445

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/edgex/edgex-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ typedef struct devsdk_nvpairs
struct devsdk_nvpairs *next;
} devsdk_nvpairs;

/**
/**
* @brief Creates a new string list, optionally adding to an existing list
* @param str The string to be added
* @param list A list that will be extended, or NULL
Expand Down
1 change: 1 addition & 0 deletions src/c/devman.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ void edgex_add_device

if (result)
{
devsdk_add_new_device (svc->add_device_new, name);
iot_log_info (svc->logger, "Device %s added with id %s", name, result);
free (result);
}
Expand Down
7 changes: 6 additions & 1 deletion src/c/devmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ static void add_locked (edgex_devmap_t *map, const edgex_device *newdev)
edgex_device *dup = edgex_device_dup (newdev);
atomic_store (&dup->refs, 1);
dup->ownprofile = false;
bool result = false;
edgex_deviceprofile **pp = edgex_map_get (&map->profiles, dup->profile->name);
if (pp)
{
Expand All @@ -99,7 +100,11 @@ static void add_locked (edgex_devmap_t *map, const edgex_device *newdev)
edgex_map_set (&map->profiles, dup->profile->name, dup->profile);
}
edgex_map_set (&map->devices, dup->name, dup);
edgex_device_autoevent_start (map->svc, dup);
result = search_devsdk_new_device (map->svc->add_device_new, dup->name);
if (result)
{
edgex_device_autoevent_start (map->svc, dup);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what effect this is intended to have - looks as though autoevents will only be started for devices added via edgex_add_device?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what effect this is intended to have - looks as though autoevents will only be started for devices added via edgex_add_device?

@iain-anderson Thanks for the review... I am putting my observation and analysis below point wise:

The autoevents gets invoked in 2 ways for 2 different scenarios:

1. When we boot target for the very first time:
In this scenario the device profile and default device interfaces (viz. eth0 in this case) gets added to metadata for the very first time.
After the successful addition of the device the autoevents gets scheduled and invoked from the API's in the below order:

edgex_devmap_populate_devices---->add_locked---->edgex_device_autoevent_start---->ae_runner

2. When we reboot the target or poweroff and start the target:
In this scenario the device profile and default device interfaces (viz. eth0 in this case) is already in the entry and hence when sdk tries
to add the device in metadata, it goes for a check initially where it is found that device already exits in the entry and hence the device
profile and device just needs to be updated in the sdk in order to invoke autoevents with the help of device profile update callback.

Issue in target machine:
It runs fine when target gets booted for the very first time. But after reboot a flaky issue is found. Only 1st autoevent is invoked and rest
all of them are not appearing. Hence, in this case when we try to acquire value of rest of the autoevent resources from edgex developer's UI
we get 502 error.

Root cause of the issue:
After reboot when service starts, sometimes the below order of API's gets triggered first before the device exist check while device addition.

edgex_devmap_populate_devices---->add_locked--->edgex_device_autoevent_start---->ae_runner

Since the device is already in metadata even before the existing check, the above API order tries to invoke the autoevents for the device with schedulers.
The ae_runner works in schedulers running parallely. Meanwhile there is an attempt for adding device where it is found under the check that
device already exists. Under the same check there is an API edgex_device_release call which has call for edgex_device_autoevent_stop inside
which actually stops and deletes the remaining autoevent schedulers which are yet to be run. Once stopped further no autoevents gets triggered ever.
In this scenario no device profile update callback gets invoked which also triggers the autoevents.

Possible fix:
We can make a data structure which would contain the list of only newly added device. We will do a search operation on the data structure just before edgex_device_autoevent_start
during the API order call as mentioned above earlier. This will make sure that autoevents gets invoked with this API order call only for newly added device.

edgex_devmap_populate_devices---->add_locked---->search_devsdk_device--->{if search for a new device successful}---->edgex_device_autoevent_start---->ae_runner

For devices already in the metadata entry, the autoevents will be automatically scheduled and invoked by the device profile update callback.

}
}

void edgex_devmap_populate_devices
Expand Down
31 changes: 31 additions & 0 deletions src/c/edgex-rest.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,37 @@ static JSON_Value *strings_to_array (const devsdk_strings *s)
return result;
}

void devsdk_add_new_device (iot_data_t *devices, const char *name)
{
if(devices == NULL)
{
devices = iot_data_alloc_map (IOT_DATA_STRING);
}
/*Check if memory got allocated or already allocated*/
if(devices)
{
iot_data_map_add (devices, iot_data_alloc_string (name, IOT_DATA_COPY), iot_data_alloc_null ());
}
}

void devsdk_clean_new_device (iot_data_t *devices)
{
if(devices)
{
iot_data_free (devices);
devices = NULL;
}
}

bool search_devsdk_new_device (iot_data_t *devices, char *name)
{
if (devices && iot_data_string_map_get (devices, name))
{
return true;
}
return false;
}

devsdk_strings *devsdk_strings_dup (const devsdk_strings *strs)
{
devsdk_strings *result = NULL;
Expand Down
3 changes: 3 additions & 0 deletions src/c/edgex-rest.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include "edgex2.h"
#include "rest-server.h"

void devsdk_add_new_device (iot_data_t *devices, const char *name);
void devsdk_clean_new_device (iot_data_t *devices);
bool search_devsdk_new_device (iot_data_t *devices, char *name);
devsdk_strings *devsdk_strings_dup (const devsdk_strings *strs);
void devsdk_strings_free (devsdk_strings *strs);
char *devsdk_nvpairs_write (const devsdk_nvpairs *e);
Expand Down
2 changes: 2 additions & 0 deletions src/c/service.c
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ static void startConfigured (devsdk_service_t *svc, const devsdk_timeout *deadli
}

edgex_devmap_populate_devices (svc->devices, devs);
devsdk_clean_new_device(svc->add_device_new);
edgex_device_free (svc, devs);

/* Start REST server now so that we get the callbacks on device addition */
Expand Down Expand Up @@ -801,6 +802,7 @@ void devsdk_service_start (devsdk_service_t *svc, iot_data_t *driverdfls, devsdk
toml_table_t *configtoml = NULL;
bool uploadConfig = false;
iot_data_t *configmap;
svc->add_device_new = NULL;

if (svc->starttime)
{
Expand Down
1 change: 1 addition & 0 deletions src/c/service.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ struct devsdk_service_t
iot_threadpool_t *thpool;
iot_threadpool_t *eventq;
iot_scheduler_t *scheduler;
iot_data_t *add_device_new;
};

extern void devsdk_schedule_metrics (devsdk_service_t *svc);
Expand Down