Skip to content

refactor: Avoid generics overuse for database - Database sessions - #292

Merged
Thorinwasher merged 3 commits into
masterfrom
refactor/database-handling
Jun 29, 2026
Merged

refactor: Avoid generics overuse for database - Database sessions#292
Thorinwasher merged 3 commits into
masterfrom
refactor/database-handling

Conversation

@Thorinwasher

Copy link
Copy Markdown
Contributor

No description provided.

@Thorinwasher
Thorinwasher force-pushed the refactor/database-handling branch 4 times, most recently from 4bbb7d7 to 51d0413 Compare June 28, 2026 09:26
@Thorinwasher
Thorinwasher force-pushed the refactor/database-handling branch from 51d0413 to 5f35d9a Compare June 28, 2026 10:04
@Mitality
Mitality self-requested a review June 29, 2026 06:47
@Mitality

Copy link
Copy Markdown
Collaborator

Don't see any major issues, though there is one thing we might want to take a closer look at;

At lines 153 and 156 in DrunksManagerImpl, we enqueue db tasks in a CompletableFuture#thenAcceptAsync block. I could be wrong here, but I think this could technically run after Database#flush on shutdown if we're unlucky with the timing

@Thorinwasher

Thorinwasher commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Don't see any major issues, though there is one thing we might want to take a closer look at;

At lines 153 and 156 in DrunksManagerImpl, we enqueue db tasks in a CompletableFuture#thenAcceptAsync block. I could be wrong here, but I think this could technically run after Database#flush on shutdown if we're unlucky with the timing

You mean after this line gets called?

🤔 I think one of the requirements for those methods in drunkenstate to be run is that the server is ticking. I basically assume this order of operation for shutdown:

  • Kick all players
  • Stop server ticking
  • Disable all plugins
  • Stop server

It might be an issue though if "stop server ticking" is run concurently with "disable all plugins", but that might apply to other database objects, as well.

Otherwise, this method should handle that, assuming no more database tasks gets submitted after or concurently when that method is called (that's because the executor is single threaded; only one thread is doing operations at the database to avoid corruption)

public CompletableFuture<Void> flush() {
return CompletableFuture.runAsync(() -> {
}, executor);
}

@Mitality

Copy link
Copy Markdown
Collaborator

You mean after this line gets called?

Yes

@Thorinwasher

Copy link
Copy Markdown
Contributor Author

You mean after this line gets called?

Yes

Ok 😛

@Mitality

Copy link
Copy Markdown
Collaborator

I think one of the requirements for those methods in drunkenstate to be run is that the server is ticking

If that's the case then this issue probably won't exist in practice- If I remember correctly, the tick loop stops before plugins are disabled and between that, there's probably also some time spent saving the world and etc.

@Mitality

Copy link
Copy Markdown
Collaborator

You mean after this line gets called?

Yes

Ok 😛

Double Ok :P

@Mitality

Copy link
Copy Markdown
Collaborator

You mean after this line gets called?

Yes

Ok 😛

Double Ok :P

Is there a limit for nested replies? 👀

@Thorinwasher

Copy link
Copy Markdown
Contributor Author

You mean after this line gets called?

Yes

Ok 😛

Double Ok :P

Is there a limit for nested replies? 👀

No

@Mitality

Copy link
Copy Markdown
Collaborator

You mean after this line gets called?

Yes

Ok 😛

Double Ok :P

Is there a limit for nested replies? 👀

No

Awesome 😄

@Thorinwasher

Copy link
Copy Markdown
Contributor Author

You mean after this line gets called?

Yes

Ok 😛

Double Ok :P

Is there a limit for nested replies? 👀

No

Awesome 😄

Perfect way to make it hard to read the actual discussion 😛

@Mitality Mitality left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I approve! :)

@Thorinwasher
Thorinwasher merged commit 1f31995 into master Jun 29, 2026
1 of 2 checks passed
@Thorinwasher
Thorinwasher deleted the refactor/database-handling branch June 29, 2026 07:48
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.

2 participants