max_error_reason_size limit error reason

[kasimtj]

Description

example of limiting error reason to solve large tmp file reading problem

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

Checklist

• Linter passes correctly
• Add tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary

Does this introduce a breaking change?

• Yes
• No

Further comments

[mga-chka]

FYI all the maintainers are off, we'll review the PR in January

[mga-chka]

Hi, thanks for the PR.
Can you add documentations and tests to your PR?
You can look at what was done in the PR of the max_payload_side to help you in these tasks

[mga-chka]

Can you add a default value at "" in order to avoid a nil errReason? It could lead to future bugs

[kasimtj]

added default value

[mga-chka]

on top of that, please do run the tests on your laptop because your PR broke some existing tests:

=== RUN   TestLoadConfig/full_description
    config_test.go:341: unexpected config result. Diff:   []uint8(
          	"""
          	... // 123 identical lines
          	  networks:
          	  - 10.10.10.0/24
        - 	max_error_reason_size: 1125899906842624
          	caches:
          	- mode: file_system
          	... // 44 identical lines
          	"""
          )
=== RUN   TestLoadConfig/default_values
    config_test.go:341: unexpected config result. Diff:   []uint8(
          	"""
          	... // 25 identical lines
          	  to_user: default
          	  max_execution_time: 2m
        - 	max_error_reason_size: 1125899906842624
          	"""
          )
--- FAIL: TestLoadConfig (0.01s)
    --- FAIL: TestLoadConfig/full_description (0.01s)
    --- FAIL: TestLoadConfig/default_values (0.00s)```

[kasimtj]

on top of that, please do run the tests on your laptop because your PR broke some existing tests:

=== RUN   TestLoadConfig/full_description
    config_test.go:341: unexpected config result. Diff:   []uint8(
          	"""
          	... // 123 identical lines
          	  networks:
          	  - 10.10.10.0/24
        - 	max_error_reason_size: 1125899906842624
          	caches:
          	- mode: file_system
          	... // 44 identical lines
          	"""
          )
=== RUN   TestLoadConfig/default_values
    config_test.go:341: unexpected config result. Diff:   []uint8(
          	"""
          	... // 25 identical lines
          	  to_user: default
          	  max_execution_time: 2m
        - 	max_error_reason_size: 1125899906842624
          	"""
          )
--- FAIL: TestLoadConfig (0.01s)
    --- FAIL: TestLoadConfig/full_description (0.01s)
    --- FAIL: TestLoadConfig/default_values (0.00s)```

fixed all tests, added new ones

[kasimtj]

TestKillQuery::timeout_cluster_user runs well on my local machine

[mga-chka]

don't worry, this test is sometimes fails only on github (there is a PR to fix that).
You fixed the failing test. Once you add new tests and documentations (cf my previous comment), we'll be able to merge your PR

[kasimtj]

don't worry, this test is sometimes fails only on github (there is a PR to fix that).
You fixed the failing test. Once you add new tests and documentations (cf my previous comment), we'll be able to merge your PR

I added new case for TestReverseProxy_ServeHTTP1 and checks for transaction fail reasons for all other cases
Can you clarify please which test cases you want me to cover more?

Also I added config into default.md

[kasimtj]

also 2 days ago we deployed this branch in our production and solved all OOM problems, we will go back to your upstream as soon as PR will be merged

[mga-chka]

Hi, I'm a bit busy, I'll review your PR next week. If it's ok, I'll merge it then release a new version of chproxy.

[mga-chka]

looks good to me. I don't merge the PR so that the other reviewers can review it if they want.
We'll release a new version containing your fix soon (before next week)