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

Revert "Prevent directory modifications when iterating" #518

Merged

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Nov 14, 2023

Reverts #515

The upgrade fails due to memory exhaustion on step 9 (removing files).

Fixes #517

@nickvergessen
Copy link
Member

cc @caco3 sadly had to revert it due to memory issues (512MB were not enough to update our instance)

@blizzz blizzz merged commit 4d660d8 into master Nov 14, 2023
22 of 23 checks passed
@blizzz blizzz deleted the revert-515-prevent-directory-modifications-when-iterating2 branch November 14, 2023 22:11
@caco3
Copy link

caco3 commented Nov 14, 2023

@nickvergessen Thanks for the info. That is a pity.
Do you know why it runs out of memory?
Is the data dir part of the root dir? Do you have a lot of files in there?
On my system, I have only 256 MB RAM and it still works.
It would be helpful if I could provoke this issue on my system so I can investigate.

Maybe once #507 or #516 gets solved, #515 would be fine?

@blizzz Do you think we could have a look on using my originally proposed recursive rrmdir?

@blizzz
Copy link
Member Author

blizzz commented Nov 14, 2023

We think the apps folder be the culprit with almost 70k file. 512 MB were exceeded. The data dir is elsewhere.

@caco3
Copy link

caco3 commented Nov 14, 2023

hmm, but why did this not get catched on the checks? Do you have higher memory limits there?
By default, the app folder has around 13k of files, do you have a lot of extra apps installed?

@nickvergessen
Copy link
Member

hmm, but why did this not get catched on the checks? Do you have higher memory limits there?

Well this was with the CLI updater, so all steps execute in one process?

By default, the app folder has around 13k of files, do you have a lot of extra apps installed?

Quite some yeah, i guess

{
    "enabled": {
        "activity": "2.20.0",
        "admin_audit": "1.18.0",
        "announcementcenter": "6.7.0",
        "approval": "1.1.0",
        "bruteforcesettings": "2.8.0",
        "calendar": "4.6.0-beta.1",
        "calendar_resource_management": "0.6.0-alpha.1",
        "call_summary_bot": "1.0.0",
        "circles": "28.0.0-dev",
        "cloud_federation_api": "1.11.0",
        "collectives": "2.9.0",
        "comments": "1.18.0",
        "contacts": "5.5.0-beta.4",
        "contactsinteraction": "1.9.0",
        "dashboard": "7.8.0",
        "dav": "1.29.1",
        "deck": "1.12.0-beta.3",
        "droneci_fast_lane": "1.3.0",
        "external": "5.3.1",
        "federatedfilesharing": "1.18.0",
        "federation": "1.18.0",
        "files": "2.0.0",
        "files_automatedtagging": "1.18.0",
        "files_external": "1.20.0",
        "files_lock": "28.0.0-beta.1",
        "files_pdfviewer": "2.9.0",
        "files_reminders": "1.0.0",
        "files_sharing": "1.20.0",
        "files_trashbin": "1.18.0",
        "files_versions": "1.21.0",
        "firstrunwizard": "2.17.0",
        "forms": "3.3.1",
        "groupfolders": "16.0.0",
        "guests": "3.0.0",
        "integration_giphy": "1.0.8",
        "integration_github": "2.0.6",
        "integration_mastodon": "2.0.3",
        "integration_openstreetmap": "1.0.6",
        "integration_zammad": "2.0.6",
        "llm": "1.2.0",
        "logreader": "2.13.0",
        "lookup_server_connector": "1.16.0",
        "mail": "3.5.0-beta.3",
        "nextcloud_announcements": "1.17.0",
        "notes": "4.9.0-beta.2",
        "notifications": "2.16.0",
        "notify_push": "0.6.5",
        "oauth2": "1.16.3",
        "password_policy": "1.18.0",
        "photos": "2.4.0",
        "privacy": "1.12.0",
        "provisioning_api": "1.18.0",
        "related_resources": "1.3.0",
        "richdocuments": "8.3.0-beta.1",
        "sentry": "8.8.11",
        "settings": "1.10.0",
        "sharebymail": "1.18.0",
        "sketch_picker": "1.0.1",
        "spreed": "18.0.0-beta.2",
        "stt_whisper": "1.0.7",
        "support": "1.11.0",
        "survey_client": "1.16.0",
        "suspicious_login": "6.0.0",
        "systemtags": "1.18.0",
        "tables": "0.6.2",
        "tasks": "0.15.0",
        "text": "3.9.0",
        "text_templates": "1.0.4",
        "theming": "2.3.0",
        "translate": "1.2.0",
        "twofactor_backupcodes": "1.17.0",
        "twofactor_nextcloud_notification": "3.8.0",
        "twofactor_totp": "10.0.0-beta.2",
        "twofactor_webauthn": "1.3.0-alpha.1",
        "updatenotification": "1.18.0",
        "user_ldap": "1.19.0",
        "user_status": "1.8.1",
        "viewer": "2.2.0",
        "weather_status": "1.8.0",
        "workflowengine": "2.10.0"
    },
    "disabled": {
        "encryption": "2.16.0",
        "end_to_end_encryption": "1.13.1 (installed 1.13.1)",
        "files_antivirus": "5.3.0 (installed 5.3.0)",
        "files_markdown": "2.4.1 (installed 2.4.1)",
        "files_retention": "1.16.0 (installed 1.16.0)",
        "files_rightclick": "0.15.1 (installed 1.6.0)",
        "integration_discourse": "2.0.4 (installed 2.0.4)",
        "polls": "6.0.0-beta2 (installed 6.0.0-beta2)",
        "recognize": "5.0.3 (installed 5.0.3)",
        "recommendations": "2.0.0 (installed 1.2.0)",
        "serverinfo": "1.18.0 (installed 1.12.0)",
        "socialsharing_diaspora": "3.0.0 (installed 3.0.0)",
        "socialsharing_email": "3.0.0 (installed 3.0.0)",
        "socialsharing_facebook": "3.0.0 (installed 3.0.0)",
        "socialsharing_twitter": "3.0.0 (installed 3.0.0)",
        "talk_simple_poll": "0.1.0 (installed 0.1.0)",
        "twofactor_admin": "4.3.0 (installed 4.3.0)",
        "users_picker": "0.2.2 (installed 0.2.2)"
    }
}

@marcelstoer
Copy link

marcelstoer commented Nov 15, 2023

What a pity 😢

What is the plan now to address #158 (should be reopened I guess)?

All of that before v28 is released?

@caco3
Copy link

caco3 commented Nov 15, 2023

Well this was with the CLI updater, so all steps execute in one process?

No, I always use the Web UI.

I can confirm that there is an error with too many files. I created 500 extra folders in apps and added 120 files in each of those folders using

for i in {001..500}; do    mkdir -p $i;  echo $i; for f in {001..120}; do echo "$i-$f" > $i/$f;   done;    done

This throws an error 500 in step 9.

@nickvergessen
Copy link
Member

What is the plan now to address #158 (should be reopened I guess)?

I would say reopen a PR with a revert of the revert and try to fix the problem there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Out of memory with new directory iterator code
4 participants