Implement daemon mode to interop with paperd #2836
Conversation
DenWav
changed the title
|
Hello, do you have any update on third party testing? could i help somehow? Thanks Michael |
DenWav
force-pushed
the
feature/daemon
branch
3 times, most recently
from
96533e0
to
0d83016
Compare
|
@aikar @zachbr @electronicboy if y'all could take a gander I'd appreciate it - it's somewhat of a large change but 99% of it is all new code. |
DenWav
changed the title
DenWav
force-pushed
the
feature/daemon
branch
4 times, most recently
from
2c787a8
to
bb6d65d
Compare
paperd source code:
https://github.com/PaperMC/paperd
Information about the paperd protocol:
https://github.com/PaperMC/paperd/blob/master/protocol.md
DenWav
changed the title
| + <dependency> | ||
| + <groupId>org.apache.logging.log4j</groupId> | ||
| + <artifactId>log4j-core</artifactId> | ||
| + <version>2.12.0</version> |
There was a problem hiding this comment.
We already have log4j-core for 2.8.1 defined; do we need 2.12 here?
There was a problem hiding this comment.
I have no clue why that's there honestly. I'll need to look again.
| + /** | ||
| + * This is the Unix socket file that {@code paperd} will use to communicate with this server. | ||
| + */ | ||
| + private static final String SOCK_FILE = "paper.sock"; |
There was a problem hiding this comment.
Customisable perhaps? Most software which operates using sockets allows at least for some env-var to set the socket path.
There was a problem hiding this comment.
That's a good thought, easy to add in too.
| + */ | ||
| + private static int sock; | ||
| + /** | ||
| + * IF we are in {@link #IS_DAEMON daemon mode} this will be set to the {@link Path} created from {@link #SOCK_FILE} |
There was a problem hiding this comment.
| + * IF we are in {@link #IS_DAEMON daemon mode} this will be set to the {@link Path} created from {@link #SOCK_FILE} | |
| + * If we are in {@link #IS_DAEMON daemon mode} this will be set to the {@link Path} created from {@link #SOCK_FILE} |
| + | ||
| + /* package */ MemoryStatus(final long freeMemory, final long totalMemory, final long maxMemory) { | ||
| + final long usedMemory = totalMemory - freeMemory; | ||
| + this.usedMemory = (usedMemory / 1_000_000) + " MB"; |
There was a problem hiding this comment.
These strings should probably be left to paperd.
| { | ||
| } | ||
|
|
||
| + /* Paper daemon - this is misleading, calling MinecraftServer.close above will already call System.exit |
There was a problem hiding this comment.
In that case, this seems like it's unnecessary diff.
This should probably also be reported to Spigot's JIRA.
| new file mode 100644 | ||
| index 0000000000000000000000000000000000000000..4cf198fccb1aab43930745288717fa9ad308a39b | ||
| --- /dev/null | ||
| +++ b/src/main/java/com/destroystokyo/paper/daemon/CloseableQueue.java |
There was a problem hiding this comment.
Although not really a big deal for internal classes, perhaps we should use io.papermc.paper here? The JNI part makes this change harder for us to do, as not everyone might update their paperd while updating Paper, which would break it as a package changes in the future while paperd doesn't accomodate it. paperd isn't even released yet, so it shouldn't really pose any issue to be future proof.
Proximyst
added
status: rebase required
type: feature
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Stale who? Stale you! Wonder if we can make the bot ignore some issues... |
electronicboy
added
the
status: accepted
|
FeelsWeirdMan stale bot |
Re-opened from #2319 to target
master.This commit adds the necessary logic so Paper can communicate with paperd.
paperd's 1.1.0 release marks the first release and compatible version with this PR.
Documentation on usage can be found here.
Documentation on the internals of the communication system can be found here.
Build Changes
This change also requires updates to
paperclip-maven-pluginwhich were done here.And the change to Paperclip to incorporate that Maven plugin update is here.
Both
paperclip-maven-pluginand Paperclip itself were moved to theio.papermcpackage as well as a normalSNAPSHOTversion bump.Necessary steps before merging:
Currently on hold as there's a major bug in
paperdthat I need to fix first: PaperMC/paperd#9.