2023 slow statements fix #195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fearoffish
force-pushed
the
2023-statements-fix
branch
from
2414e93
to
f94cd0f
Compare
We aggregate task events into a single task, to get around memory issues when there are millions
fearoffish
force-pushed
the
2023-statements-fix
branch
from
6dd7ce9
to
559d186
Compare
corlettb
approved these changes
Dec 6, 2023
apiserver/server_billable_handler.go
Outdated
| // Check if the resource type is "task" | ||
| row, err := rows.Event() | ||
|
|
||
| if row != nil && row.ResourceType != "" && row.ResourceType == "task" { |
There was a problem hiding this comment.
the row.ResourceType != "" is redundant if we are checking the row.ResourceType == "task"
apiserver/server_billable_handler.go
Outdated
| key := fmt.Sprintf("%s-%s", row.OrgGUID, row.SpaceGUID) | ||
|
|
||
| // Convert the price values to float | ||
| priceInc, _ := strconv.ParseFloat(row.Price.IncVAT, 128) |
There was a problem hiding this comment.
ParseFloat should only get 32 or 64... We should switch back to 64 as it only returns a float64.
fearoffish
force-pushed
the
2023-statements-fix
branch
from
e9947eb
to
047af3e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Some organisations have many many tasks in their events list, which are typically one off quick scripts. The sheer amount of them causes a memory issue for node to handle on a statements page. This fix gathers all tasks and groups them into one aggregate event with a summed cost for all of them in one.
How to review
Deploy to a dev environment and then check the statements page. This really needs a huge number of task events (say 50k a day) to really see the benefit.
Who can review
Anyone who can deploy the app.
🚨⚠️ Please do not merge this pull request via the GitHub UI ⚠️ 🚨