Add DB table to extend filecache with metadata etag, creation and upload time

[MorrisJobke]

For #8477

Now the big question is: should we try to add more columns to this new table. The idea was to add some generic columns for strings, integer, bools, etc that are by default are just empty and can be used in the future then? The problem we want to solve with this is that the filecache is a big table that is not easily extendable on all possible DBs without a major downtime due to the size of the table. This is especially the case for MySQL/MariaDB where an added column results in the table being created from scratch and then is copied over and thus causing quite some downtime.

cc @rullzer @icewind1991 @karlitschek @nickvergessen @blizzz @kesselb @ChristophWurst

[karlitschek]

I think it would be clever to add a few 'spare' columns. It will basically be impossible to change this table in the future once it has millions of rows. So I think this is a good idea.

[nickvergessen]

We had a few columns to add already. ctime/utime/mtime need to be covered, we only got 1or2 so far.

The question with empty columns is of course, do we need more int or strings?

[MorrisJobke]

The question with empty columns is of course, do we need more int or strings?

Another one: what size should they have, because there are multiple sizes on the int side as well as a multitude on the string side as well. Just adding "string columns" isn't that easy because it has given implications on the performance (how easy it is to add indexes, or if this is possible at all, etc).

We had a few columns to add already. ctime/utime/mtime need to be covered, we only got 1or2 so far.

Good ones 👍 I would then add a creation time and an upload time column as well, right?

[MorrisJobke]

Any other columns? Something like the visibility due to background scans or background processing? (I just have the antivirus app in mind for example)

[MorrisJobke]

The problem we want to solve with this is that the filecache is a big table that is not easily extendable on all possible DBs without a major downtime due to the size of the table. This is especially the case for MySQL/MariaDB where an added column results in the table being created from scratch and then is copied over and thus causing quite some downtime.

There is also a tool to migrate it on a live system: https://www.percona.com/doc/percona-toolkit/2.1/pt-online-schema-change.html (problem: it's a tool that needs manual work)

[MorrisJobke]

I added creation and upload time as datetime columns.

[MorrisJobke]

Anything else?

[MorrisJobke]

Added the index names 👍

[MorrisJobke]

Ready for review 👍

[blizzz]

will timestamps (mtime in filecache) and the datetimes here will be comparable? since the timestamp does not carry timezone information. i think @nickvergessen dived into this somewhat deeper some time ago?

[MorrisJobke]

will timestamps (mtime in filecache) and the datetimes here will be comparable? since the timestamp does not carry timezone information. i think @nickvergessen dived into this somewhat deeper some time ago?

timestamps are by definition UTC, right? Thus it should be no problem.

[blizzz]

@MorrisJobke I am mistaken. DateTime does not carry time zone information, but as long as data is provided in UTC it'll be fine.

[icewind1991]

since we use timestamps (int) for storing time in the filecache I would prefer to also use that here

[MorrisJobke]

since we use timestamps (int) for storing time in the filecache I would prefer to also use that here

This was proposed by @nickvergessen to improve handling of this in the future. I don’t have any specific preference for one or the other.

What do the others prefer?

[MorrisJobke]

since we use timestamps (int) for storing time in the filecache I would prefer to also use that here

Changed.

[faily-bot]

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 16749: failure

• TESTS=acceptance, TESTS-ACCEPTANCE=app-files
  • tests/acceptance/features/app-files.feature:41
  • tests/acceptance/features/app-files.feature:90

Show full log

  Scenario: viewing a favorite file in its folder shows the correct sidebar view # /drone/src/github.com/nextcloud/server/tests/acceptance/features/app-files.feature:41
    Given I am logged in                                                         # LoginPageContext::iAmLoggedIn()
    And I create a new folder named "other"                                      # FileListContext::iCreateANewFolderNamed()
    And I mark "other" as favorite                                               # FileListContext::iMarkAsFavorite()
    And I mark "welcome.txt" as favorite                                         # FileListContext::iMarkAsFavorite()
    And I see that "other" is marked as favorite                                 # FileListContext::iSeeThatIsMarkedAsFavorite()
    And I see that "welcome.txt" is marked as favorite                           # FileListContext::iSeeThatIsMarkedAsFavorite()
    And I open the "Favorites" section                                           # AppNavigationContext::iOpenTheSection()
    And I open the details view for "other"                                      # FileListContext::iOpenTheDetailsViewFor()
    And I see that the details view is open                                      # FilesAppContext::iSeeThatTheDetailsViewIsOpen()
    And I see that the file name shown in the details view is "other"            # FilesAppContext::iSeeThatTheFileNameShownInTheDetailsViewIs()
    When I view "welcome.txt" in folder                                          # FileListContext::iViewInFolder()
      Row for file welcome.txt in file list could not be found after 100 seconds
      File actions menu button for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)
    Then I see that the current section is "All files"                           # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    And I see that the details view is open                                      # FilesAppContext::iSeeThatTheDetailsViewIsOpen()
    And I see that the file name shown in the details view is "welcome.txt"      # FilesAppContext::iSeeThatTheFileNameShownInTheDetailsViewIs()
    When I open the details view for "other"                                     # FileListContext::iOpenTheDetailsViewFor()
    And I see that the file name shown in the details view is "other"            # FilesAppContext::iSeeThatTheFileNameShownInTheDetailsViewIs()
  Scenario: show favorites                                            # /drone/src/github.com/nextcloud/server/tests/acceptance/features/app-files.feature:90
    Given I am logged in                                              # LoginPageContext::iAmLoggedIn()
    And I mark "welcome.txt" as favorite                              # FileListContext::iMarkAsFavorite()
    When I open the "Favorites" section                               # AppNavigationContext::iOpenTheSection()
    Then I see that the current section is "Favorites"                # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    Then I see that the file list contains a file named "welcome.txt" # FileListContext::iSeeThatTheFileListContainsAFileNamed()
      Row for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)

• TESTS=acceptance, TESTS-ACCEPTANCE=login
  • tests/acceptance/features/login.feature:38

Show full log

  Scenario: log in with invalid user once fixed by admin              # /drone/src/github.com/nextcloud/server/tests/acceptance/features/login.feature:38
    Given I act as John                                               # ActorContext::iActAs()
    And I can not log in with user unknownUser and password 123456acb # LoginPageContext::iCanNotLogInWithUserAndPassword()
    When I act as Jane                                                # ActorContext::iActAs()
    And I am logged in as the admin                                   # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                      # SettingsMenuContext::iOpenTheUserSettings()
    And I click the New user button                                   # UsersSettingsContext::iClickTheNewUserButton()
    And I see that the new user form is shown                         # UsersSettingsContext::iSeeThatTheNewUserFormIsShown()
    And I create user unknownUser with password 123456acb             # UsersSettingsContext::iCreateUserWithPassword()
    And I see that the list of users contains the user unknownUser    # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I act as John                                                 # ActorContext::iActAs()
    And I log in with user unknownUser and password 123456acb         # LoginPageContext::iLogInWithUserAndPassword()
    Then I see that the current page is the Files app                 # FilesAppContext::iSeeThatTheCurrentPageIsTheFilesApp()
      Failed asserting that 'http://acceptance-login/index.php/login?user=unknownUser' starts with "http://acceptance-login/index.php/apps/files/".

• TESTS=acceptance, TESTS-ACCEPTANCE=users
  • tests/acceptance/features/users.feature:13
  • tests/acceptance/features/users.feature:25
  • tests/acceptance/features/users.feature:35
  • tests/acceptance/features/users.feature:47

Show full log

  Scenario: create a new user with a custom display name                       # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:13
    Given I am logged in as the admin                                          # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                               # SettingsMenuContext::iOpenTheUserSettings()
    When I click the New user button                                           # UsersSettingsContext::iClickTheNewUserButton()
    And I see that the new user form is shown                                  # UsersSettingsContext::iSeeThatTheNewUserFormIsShown()
    And I set the user name for the new user to "test"                         # UsersSettingsContext::iSetTheUserNameForTheNewUserTo()
    And I set the display name for the new user to "Test display name"         # UsersSettingsContext::iSetTheDisplayNameForTheNewUserTo()
    And I set the password for the new user to "123456acb"                     # UsersSettingsContext::iSetThePasswordForTheNewUserTo()
    And I create the new user                                                  # UsersSettingsContext::iCreateTheNewUser()
    Then I see that the list of users contains the user "test"                 # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I see that the display name for the user "test" is "Test display name" # UsersSettingsContext::iSeeThatTheDisplayNameForTheUserIs()
      Row for user test in Users Settings could not be found after 100 seconds
      displayName cell for user test in Users Settings could not be found after 100 seconds
      displayName input for user test in Users Settings could not be found after 100 seconds (NoSuchElementException)
  Scenario: delete a user                                                      # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:25
    Given I act as Jane                                                        # ActorContext::iActAs()
    And I am logged in as the admin                                            # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                               # SettingsMenuContext::iOpenTheUserSettings()
    And I see that the list of users contains the user user0                   # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I open the actions menu for the user user0                             # UsersSettingsContext::iOpenTheActionsMenuOf()
    And I see that the "Delete user" action in the user0 actions menu is shown # UsersSettingsContext::iSeeTheAction()
      Failed asserting that false is true.
    When I click the "Delete user" action in the user0 actions menu            # UsersSettingsContext::iClickTheAction()
    Then I see that the list of users does not contains the user user0         # UsersSettingsContext::iSeeThatTheListOfUsersDoesNotContainsTheUser()
  Scenario: disable a user                                                      # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:35
    Given I act as Jane                                                         # ActorContext::iActAs()
    And I am logged in as the admin                                             # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                                # SettingsMenuContext::iOpenTheUserSettings()
    And I see that the list of users contains the user user0                    # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I open the actions menu for the user user0                              # UsersSettingsContext::iOpenTheActionsMenuOf()
    And I see that the "Disable user" action in the user0 actions menu is shown # UsersSettingsContext::iSeeTheAction()
      Failed asserting that false is true.
    When I click the "Disable user" action in the user0 actions menu            # UsersSettingsContext::iClickTheAction()
    Then I see that the list of users does not contains the user user0          # UsersSettingsContext::iSeeThatTheListOfUsersDoesNotContainsTheUser()
    When I open the "Disabled users" section                                    # AppNavigationContext::iOpenTheSection()
    Then I see that the list of users contains the user user0                   # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
  Scenario: users navigation without disabled users                                   # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:47
    Given I act as Jane                                                               # ActorContext::iActAs()
    And I am logged in as the admin                                                   # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                                      # SettingsMenuContext::iOpenTheUserSettings()
    And I open the "Disabled users" section                                           # AppNavigationContext::iOpenTheSection()
    And I see that the list of users contains the user disabledUser                   # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I open the actions menu for the user disabledUser                             # UsersSettingsContext::iOpenTheActionsMenuOf()
    And I see that the "Enable user" action in the disabledUser actions menu is shown # UsersSettingsContext::iSeeTheAction()
      Failed asserting that false is true.
    When I click the "Enable user" action in the disabledUser actions menu            # UsersSettingsContext::iClickTheAction()
    Then I see that the section "Disabled users" is not shown                         # AppNavigationContext::iSeeThatTheSectionIsNotShown()
    When I open the User settings                                                     # SettingsMenuContext::iOpenTheUserSettings()
    Then I see that the section "Disabled users" is not shown                         # AppNavigationContext::iSeeThatTheSectionIsNotShown()

[MorrisJobke]

Big question now: Should we merge this? because the whole logic to properly provide the metadata etag is quite some work and the propagation that is done for the normal etag needs to be replicated to the sharing backend. That's most likely nothing we can accomplish in the remain days.

I would vote for moving this to 17. Sorry for the delay here.

[MorrisJobke]

Big question now: Should we merge this? because the whole logic to properly provide the metadata etag is quite some work and the propagation that is done for the normal etag needs to be replicated to the sharing backend. That's most likely nothing we can accomplish in the remain days.

I would vote for moving this to 17. Sorry for the delay here.

I updated this PR and we are now fine with merging this. Once in we can focus on the actual code for the usage of those columns.

[MorrisJobke]

Rebased. Once done I will merge this.

[tobiasKaminsky]

woho 🎉
Will the other parts also be part of NC17, like metadata etag?

[MorrisJobke]

For now it’s only the table columns. No code was added yet.

[MorrisJobke]

Most likely not, because the propagation code needs to be added for the metadata stuff.