fix: calculate push page size based on monitor size per batch #993
Conversation
There was a problem hiding this comment.
@shahzad31 Thanks for the PR, Can we change the PR based on what we discussed
decide the no of monitors based on size of monitor size while pushing
src/push/bundler.ts
Outdated
| await this.cleanup(output); | ||
| return data; | ||
| return { content, sizeKb }; |
There was a problem hiding this comment.
@shahzad31 Please review, this change of the return type is breaking unit tests
|
@shahzad31 can we adjust the PR description to match the actual intention? This doesn't seem to involve any retries as it is |
yeah i will come back to this PR next week |
shahzad31
changed the title
src/push/request.ts
Outdated
| @@ -58,6 +58,7 @@ export async function sendReqAndHandleError<T>( | |||
| options: APIRequestOptions | |||
| ): Promise<T> { | |||
| const { statusCode, body } = await sendRequest(options); | |||
|
|
|||
| return ( | |||
| await handleError(statusCode, options.url, body) | |||
There was a problem hiding this comment.
Can we add the chunk size and payload size information on this error? It will be helpful to know how big the payload was and how many monitors per chunk were sent
src/push/request.ts
Outdated
| statusCode, | ||
| options.url, | ||
| body, | ||
| `${options.body?.length} number of monitors were sent, with a total size of ${options.body?.length} bytes` |
There was a problem hiding this comment.
Feels weird, Why not Kibana API return proper error message when we go over the limits?
There was a problem hiding this comment.
kibana doesn't return the payload size
There was a problem hiding this comment.
In this case, we need to do this only for the 413 status code, Otherwise we are showing this error message for all the error codes which feels incorrect.
|
this is how error looks like now 
|
| let currentSize = 0; | ||
|
|
||
| for (const item of arr) { | ||
| const sizeKiB = item.id ? sizes.get(item.id) / 1024 : 1; |
There was a problem hiding this comment.
The size is stored in bytes, we need to convert them to kibibytes and round it off - #993 (comment)
decide the no of monitors based on size of monitor size while pushing !!