-
Notifications
You must be signed in to change notification settings - Fork 35
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
Force sync got stuck with in progress error when both force sync and maintenance sync happened together. #227
Conversation
7047039
to
5194342
Compare
@@ -282,7 +292,7 @@ void *WebConfigMultipartTask(void *status) | |||
ts.tv_sec += get_retry_timer(); | |||
WebcfgDebug("The retry triggers at %s\n", printTime((long long)ts.tv_sec)); | |||
} | |||
if(get_global_webcfg_forcedsync_needed() == 1) | |||
if((get_webcfg_forceSync_needed() == 1) || (get_global_webcfg_forcedsync_needed() == 1) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the same queued_forcesync string for getter setters also as it is getting confused with second check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this as second condition in OR
Update the PR heading with proper description |
@@ -79,6 +79,11 @@ static int g_testfile = 0; | |||
static int g_supplementarySync = 0; | |||
static int g_webcfg_forcedsync_needed = 0; | |||
static int g_webcfg_forcedsync_started = 0; | |||
|
|||
//Added for RDKB-55566 - Force sync and maintenance sync happened together. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of mentioning ticket number in code at multiple places, add only the required details. The issue details can be added in PR description also
//Added for RDKB-55566 - Force sync and maintenance sync happened together. | ||
//webcfg_forceSync_needed is set initially whenever force sync set is detected, cloud sync will be retried once maintenance sync is completed. | ||
set_webcfg_forceSync_needed(1); | ||
WebcfgInfo("set webcfg_forcesync_needed to %d\n", get_webcfg_forceSync_needed()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this as per new func name
Check why unit tests are failing and fix |
@@ -330,6 +340,14 @@ void *WebConfigMultipartTask(void *status) | |||
set_global_webcfg_forcedsync_started(1); | |||
WebcfgDebug("webcfg_forcedsync_needed reset to %d and webcfg_forcedsync_started %d\n", get_global_webcfg_forcedsync_needed(), get_global_webcfg_forcedsync_started()); | |||
} | |||
//Added for RDKB-55566 - Force sync and maintenance sync happene together. | |||
//webcfg_forceSync_needed is set initially whenever force sync SET is detected internally & webcfg_forceSync_started is set when actual sync is started once maintenance sync is completed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this comment, it is already available above. If we need to mention any specific details for this issue, then can be added
@@ -330,6 +340,14 @@ void *WebConfigMultipartTask(void *status) | |||
set_global_webcfg_forcedsync_started(1); | |||
WebcfgDebug("webcfg_forcedsync_needed reset to %d and webcfg_forcedsync_started %d\n", get_global_webcfg_forcedsync_needed(), get_global_webcfg_forcedsync_started()); | |||
} | |||
//Added for RDKB-55566 - Force sync and maintenance sync happene together. | |||
//webcfg_forceSync_needed is set initially whenever force sync SET is detected internally & webcfg_forceSync_started is set when actual sync is started once maintenance sync is completed. | |||
if(get_webcfg_forceSync_needed()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we use the same set_global_webcfg_forcedsync_started flag for this use case also?
Just by extending the OR condition in line 337 this issue can be fixed. With that way, we do not need 4 getter setter functions, try to optimize the fix by using only 1 flag and its setter getter function.
@@ -110,4 +110,9 @@ long timeVal_Diff(struct timespec *starttime, struct timespec *finishtime); | |||
void initWebConfigClient(); | |||
pthread_t get_global_client_threadid(); | |||
void JoinThread (pthread_t threadId); | |||
|
|||
int get_webcfg_forceSync_needed(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 getter setter functions are redundant , try to fix with 1 flag
No description provided.