Skip to content

prevent deleteRows of system tables - #5491

Merged
kevinrr888 merged 2 commits into
apache:mainfrom
kevinrr888:PreventDeleteRowsSysTables
Apr 23, 2025
Merged

prevent deleteRows of system tables#5491
kevinrr888 merged 2 commits into
apache:mainfrom
kevinrr888:PreventDeleteRowsSysTables

Conversation

@kevinrr888

Copy link
Copy Markdown
Member

Prevents tableOperations().deleteRows of system tables in 4.0.

2.1 system tables:

  • root <-- cannot delete rows
  • metadata <-- cannot delete rows
  • replication <-- can deleteRows

4.0 system tables:

  • root <-- before: cannot delete rows, after: same
  • metadata <-- before: cannot delete rows, after: same
  • scan_ref <-- before: can delete rows, after: cannot delete rows
  • fate <-- before: can delete rows, after: cannot delete rows

I did not target 2.1 because I was unsure if we want deleteRows for the replication table (seems like that was the intent, but table expectations changed in 4.0).

Created from discussion: #5474 (comment)

@kevinrr888 kevinrr888 added this to the 4.0.0 milestone Apr 21, 2025
@kevinrr888 kevinrr888 self-assigned this Apr 21, 2025
@dlmarion

Copy link
Copy Markdown
Contributor

I can see why we wouldn't want users to delete from the fate table. It's managed by the Manager, which is supposed to always be running. However, I'm wondering if we want to allow delete rows on the scanref table. The entries are created by the ScanServers, and I don't remember how they are cleaned up. I'm wondering if there is a case where all the ScanServers are stopped, but entries still remain in the table. Probably worth looking at this aspect to determine if deleteRows is needed for the scanRef table.

@kevinrr888

Copy link
Copy Markdown
Member Author

deleteRows is never used internally for scanref, and I don't think we'd want a user to do any cleanup manually.
https://github.com/apache/accumulo/blob/main/server/base/src/main/java/org/apache/accumulo/server/metadata/ScanServerRefStoreImpl.java
Is how cleanup is handled. A BatchWriter is used to delete entries, so I think it's safe to disallow deleteRows for scanref

@keith-turner keith-turner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5747 will eventually completely test these changes? Seems like thats the case, just wanted to make sure.

@kevinrr888

Copy link
Copy Markdown
Member Author

@keith-turner - Did you mean #5474? If so, yes that PR will test these changes

Comment thread server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java Outdated
@kevinrr888
kevinrr888 merged commit 16e8043 into apache:main Apr 23, 2025
@kevinrr888
kevinrr888 deleted the PreventDeleteRowsSysTables branch April 23, 2025 14:10
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.

4 participants