Skip to content
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

[chore] Silence memlimit package #3002

Merged
merged 2 commits into from
Jun 13, 2024
Merged

[chore] Silence memlimit package #3002

merged 2 commits into from
Jun 13, 2024

Conversation

daenney
Copy link
Member

@daenney daenney commented Jun 12, 2024

Description

The memlimit package started to log any error returned by automemlimit. This updates our implementation to call SetGoMemLimitWithOpts() instead which uses the same defaults as automemlimit except for being initialised with a noop logger.

We check the returned error for a particular substring, as when cgroups isn't available even when running on a Linux system that's not a problem. If it's anything but that error, we log it at the warning level so that admins can still diagnose other cgroup related issues.

Fixes #2983

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have not leveraged AI to create the proposed changes.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

The memlimit package started to log any error returned by automemlimit.
This updates our implementation to call SetGoMemLimitWithOpts() instead
which uses the same defaults as automemlimit except for being
initialised with a noop logger.

We check the returned error for a particular substring, as when cgroups
isn't available even when running on a Linux system that's not a
problem. If it's anything but that error, we log it at the warning
level so that admins can still diagnose other cgroup related issues.

Fixes #2983
@daenney daenney force-pushed the silence-memlimit branch from b4a53b0 to 86e7fcc Compare June 12, 2024 16:42
@daenney
Copy link
Member Author

daenney commented Jun 12, 2024

The test failure seems unrelated to this change:

timestamp="12/06/2024 16:51:13.997" func=dereferencing.(*Dereferencer).getAccountByURI level=ERROR msg="error enriching remote account: enrichAccount: error dereferencing https://example.org/users/Some_User: Dereference: GET request to https://example.org/users/Some_User failed: status=\"\" body=\"{\"error\":\"404 not found\"}\""

timestamp="12/06/2024 16:51:17.675" func=workers.(*clientAPI).AcceptFollow level=ERROR msg="error notifying follow: notifyFollow: error notifying follow target 01F8MH17FWEB39HZJ76B6VXSKF: Notify: couldn't retrieve mutes for account 01F8MH17FWEB39HZJ76B6VXSKF: sqlite3: database schema has changed: no such table: user_mutes (code=17 extended=17)"

timestamp="12/06/2024 16:51:17.676" func=workers.(*clientAPI).UndoFollow level=ERROR msg="error updating account stats: decrementFollowingCount: db error updating account stats: sql: database is closed"

timestamp="12/06/2024 16:51:17.676" func=workers.(*clientAPI).UndoFollow level=ERROR msg="error updating account stats: decrementFollowersCount: db error updating account stats: sql: database is closed"

timestamp="12/06/2024 16:51:17.691" func=dereferencing.(*Dereferencer).enrichAccountSafely level=ERROR msg="error updating %s fetched_at: %vhttp://fossbros-anonymous.io/users/foss_satancontext canceled"

timestamp="12/06/2024 16:51:17.691" func=dereferencing.(*Dereferencer).RefreshAccountAsync.func1 level=ERROR msg="error enriching remote account: enrichAccount: error dereferencing http://fossbros-anonymous.io/users/foss_satan: Dereference: GET request to http://fossbros-anonymous.io/users/foss_satan failed: status=\"\" body=\"{\"error\":\"404 not found\"}\""

--- FAIL: TestFromFederatorTestSuite (6.36s)
    --- FAIL: TestFromFederatorTestSuite/TestMoveAccount (3.68s)
        fromfediapi_test.go:635: 
            	Error Trace:	/drone/src/internal/processing/workers/fromfediapi_test.go:635
            	Error:      	Should be true
            	Test:       	TestFromFederatorTestSuite/TestMoveAccount
FAIL

FAIL	github.com/superseriousbusiness/gotosocial/internal/processing/workers	67.873s

Weirdly enough this test passes for me locally with the wasmsqlite3 database driver but not the modernc one. Which just. How.

@tsmethurst tsmethurst merged commit 3c86bd8 into main Jun 13, 2024
3 checks passed
@tsmethurst tsmethurst deleted the silence-memlimit branch June 13, 2024 17:02
nyarla pushed a commit to nyarla/gotosocial-modded that referenced this pull request Jun 19, 2024
The memlimit package started to log any error returned by automemlimit.
This updates our implementation to call SetGoMemLimitWithOpts() instead
which uses the same defaults as automemlimit except for being
initialised with a noop logger.

We check the returned error for a particular substring, as when cgroups
isn't available even when running on a Linux system that's not a
problem. If it's anything but that error, we log it at the warning
level so that admins can still diagnose other cgroup related issues.

Fixes superseriousbusiness#2983

Co-authored-by: tobi <31960611+tsmethurst@users.noreply.github.com>
nyarla pushed a commit to nyarla/gotosocial-modded that referenced this pull request Jun 19, 2024
The memlimit package started to log any error returned by automemlimit.
This updates our implementation to call SetGoMemLimitWithOpts() instead
which uses the same defaults as automemlimit except for being
initialised with a noop logger.

We check the returned error for a particular substring, as when cgroups
isn't available even when running on a Linux system that's not a
problem. If it's anything but that error, we log it at the warning
level so that admins can still diagnose other cgroup related issues.

Fixes superseriousbusiness#2983

Co-authored-by: tobi <31960611+tsmethurst@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] new cgroups-related error on start
2 participants