Skip to content

Admin tool event cleanup - #4935

Closed
nandorKollar wants to merge 2 commits into
apache:mainfrom
nandorKollar:admin_tool_event_cleanup
Closed

Admin tool event cleanup#4935
nandorKollar wants to merge 2 commits into
apache:mainfrom
nandorKollar:admin_tool_event_cleanup

Conversation

@nandorKollar

Copy link
Copy Markdown
Contributor

Draft for event cleanup command in admin tool. The maintenance command can be extended with further subcommands for other entity cleanup tasks, like for example metrics.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

@dimas-b dimas-b 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.

Hi @nandorKollar , I know the PR is a draft, but I could not resist posting a couple of comments about the general module structure 🙂

import java.time.Instant;

/** Maintenance operations for Polaris storage backends. */
public interface MaintenanceManager {

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.

It does not look like this interface is used in polaris-core... what's the rationale for defining it there? 🤔

If it's only that polaris-core is the only common module between Persistence and Runtime, why not add a new small module dedicated to maintenance interfaces?

From my POV overloading polaris-core with SPI classes is not ideal. Core is more of a utility library than a service. WDYT?

* Purge events older than the given cutoff from the event store. Events with a timestamp before
* {@code cutoff} are deleted. Pass {@link Instant#now()} to purge all events.
*/
void purgeEvents(Instant cutoff);

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.

Since it's about events, why not use a more specific name for the interface? e.g. EventsMaintenanceManager?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For now, it is only about events, yes. But in the future, I think we can add additional maintenance related operations here, like for example metrics related methods.

@nandorKollar

Copy link
Copy Markdown
Contributor Author

Hi @nandorKollar , I know the PR is a draft, but I could not resist posting a couple of comments about the general module structure 🙂

That's one reason why I opened it as a draft PR: to get some thoughts and ideas. 🙂 Appreciate any feedback!

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions Bot added the stale label Aug 1, 2026
@github-actions github-actions Bot closed this Aug 7, 2026
@github-project-automation github-project-automation Bot moved this from PRs In Progress to Done in Basic Kanban Board Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants