RHIROS-1327 - Adding support for report having more than 24hrs o… #122
Conversation
|
@patilsuraj767 - could you update ticket ID to RHIROS-1327 in commit message as well as title. |
patilsuraj767
force-pushed
the
multi-day-support
branch
from
510d846 to
bdb19a6
Compare
patilsuraj767
changed the title
| for _, chunk := range k8s_object_chunks { | ||
| usage_data_byte, err := kruize.Update_results(experiment_name, chunk) | ||
| if err != nil { | ||
| log.Error(err) |
There was a problem hiding this comment.
Could you confirm what details present under err? Will it be sufficient for debugging?
If something went wrong/failure occurs for update_results api with certain chunk, there will be data loss. Am I correct?
If this is so, is it expected behavior?
There was a problem hiding this comment.
Would a chunk identifier be useful here 🤔
## pseudocode
usage_data_byte, err := kruize.Update_results(experiment_name, chunk)
if err != nil {
log.Error(err, experiment_name, chunk_serial)
}
...
...
chunk_serial += 1
There was a problem hiding this comment.
Yes, along with unique identifier for resource, it is good to include chunk specific info. example - time duration
There was a problem hiding this comment.
Yes @kgaikwad if something went wrong/failure occurs for update_results api with certain chunk, there will be data loss but we cannot help it. example - if suppose kruize application is down then update_results api will fail and we cannot do anything here to stop further API requests.
There was a problem hiding this comment.
Yes are not dividing chunks based on time stamp, means it is not the case that every metrics in the chunk have the same time stamp. So I think timestamp cannot be the unique identifier.
There was a problem hiding this comment.
Yes @kgaikwad if something went wrong/failure occurs for update_results api with certain chunk, there will be data loss but we cannot help it. example - if suppose kruize application is down then update_results api will fail and we cannot do anything here to stop further API requests.
In my opinion, we should think on such scenarios and come with handling to minimize data loss. May be not for this PR but it is better to do brainstorming around.
So I think timestamp cannot be the unique identifier.
Then could you please include experiment name and any field info that might be specific to that request? So it will be easy for identifying that this upload request not processed completely, data loss observed.
There was a problem hiding this comment.
I have added the experiment_name in the log and by default request_id will be getting logged with the every log message.
There was a problem hiding this comment.
@patilsuraj767, for first part, could you add a backlog ticket so that it won't get ignored by time. Thanks!
There was a problem hiding this comment.
| for _, chunk := range k8s_object_chunks { | ||
| usage_data_byte, err := kruize.Update_results(experiment_name, chunk) | ||
| if err != nil { | ||
| log.Error(err) |
There was a problem hiding this comment.
Would a chunk identifier be useful here 🤔
## pseudocode
usage_data_byte, err := kruize.Update_results(experiment_name, chunk)
if err != nil {
log.Error(err, experiment_name, chunk_serial)
}
...
...
chunk_serial += 1
kgaikwad
changed the title
…f data.