Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webhook timings issue - Paper 1.17.1 #68

Closed
c1oneman opened this issue Jul 10, 2021 · 19 comments
Closed

Webhook timings issue - Paper 1.17.1 #68

c1oneman opened this issue Jul 10, 2021 · 19 comments

Comments

@c1oneman
Copy link
Contributor

Webhooks seem to be causing a massive performance impact on TPS.
Provided below is the timings report from paper.
https://timings.aikar.co/?id=8eb8bf944f3d4effab4f48782bbd96e3

@phybros
Copy link
Collaborator

phybros commented Sep 13, 2021

@c1oneman can you try this again with the latest version, and if possible can you capture

  1. The timings info again
  2. Your config.yml (don't include anything sensitive!)
  3. Your server startup log

@phybros
Copy link
Collaborator

phybros commented Dec 16, 2021

@c1oneman can you try this again with ServerTap v0.2.0, and provide the above info?

@phybros
Copy link
Collaborator

phybros commented Jul 14, 2022

@c1oneman can you try this again with ServerTap v0.3.0, and provide the above info?

@LittleChest
Copy link

LittleChest commented Feb 22, 2023

I have the same problem with Purpur 1.19.3 (Paper's fork) and ServerTap v0.4.0
Spark: https://spark.lucko.me/fjH9O4vTfx
ServerTap config:

port: 20004
debug: false
useKeyAuth: true
key: 'Pure' 

@phybros
Copy link
Collaborator

phybros commented Apr 11, 2023

Can you try again with v0.5.0 @LittleChest

@LittleChest
Copy link

Can you try again with v0.5.0 @LittleChest

OK! However, this is a probabilistic event, so it may take several weeks to draw a conclusion.

@LittleChest
Copy link

Can you try again with v0.5.0 @LittleChest

did not solve the problem
image

@MeesJ
Copy link

MeesJ commented May 18, 2023

I can reproduce this, with the same data in the thread dump as @LittleChest's screenshot shows.

@phybros
Copy link
Collaborator

phybros commented May 19, 2023

Yes I believe I fixed it in this commit: 88f45c6

The change prevents an additional lookup by hostname which can fail for IPv6 addresses.

I can make a release of 0.5.3 soon.

Some more details on Discord: https://discord.com/channels/919982507271802890/919982507271802893/1102610277276582019

@phybros
Copy link
Collaborator

phybros commented May 19, 2023

@MeesJ Can you reliably repro it? If so maybe I'll send you a one off jar to test if that's ok

@MeesJ
Copy link

MeesJ commented May 19, 2023

@MeesJ Can you reliably repro it? If so maybe I'll send you a one off jar to test if that's ok

Yes, for specific users this issue always happens. So if you'd consider that as reliably reproducing it, feel free to send me the test jar. I'll be more than happy to test if it fixes the issue!

Bit off-topic, but wouldn't it be even better to offer hostname lookup as a setting in the config.yml? For people who just want people's raw IP addresses instead of the rDNS value of their IP addresses, it'd be more efficient to be able to disable this mechanism entirely.

@phybros
Copy link
Collaborator

phybros commented May 19, 2023

Cool. I would bet those specific users are using IPv6!

I'll see if I can send a build shortly

@MeesJ
Copy link

MeesJ commented May 19, 2023

Cool. I would bet those specific users are using IPv6!

That doesn't seem to be the case. since they're playing via IPv4 from what it looks like. I'm playing via IPv6 myself and I do not cause any lookup issues.

@LittleChest
Copy link

Cool. I would bet those specific users are using IPv6!

I'll see if I can send a build shortly

No, I even disabled IPv6 in the network settings to prevent the server from getting IPv6.
Feel free to send a beta version if you like. ฅ⁠^⁠•⁠ﻌ⁠•⁠^⁠ฅ

@phybros
Copy link
Collaborator

phybros commented May 24, 2023

Yeah after more digging it may just be related to the reverse lookups.

To be clear, I'm not doing anything to explicitly do reverse lookups, it's just a behavior of getHostName(). So replacing it with getHostString() should prevent lookups per the docs:

Returns the hostname if known, or the result of InetAddress.getHostAddress. Unlike #getHostName, this method will never cause a DNS lookup.

@LittleChest and @MeesJ can you both try this jar? (I had to upload it here as a zip due to Github restrictions)

ServerTap-0.5.3-SNAPSHOT.jar.zip

@MeesJ
Copy link

MeesJ commented May 24, 2023

Thank you very much! I have installed the snapshot. I'll get back to you with the results soon!

@MeesJ
Copy link

MeesJ commented May 24, 2023

It looks like the snapshot has fixed the issue for me. I also like it that you changed the address field to no longer trigger a rDNS lookup!

Is it okay for you that I keep this snapshot for now? It's running smoothly for both IPv4 and IPv6 users.

@phybros
Copy link
Collaborator

phybros commented May 26, 2023

Yeah for sure. I'll do a proper release of 0.5.3 very soon too.

@phybros
Copy link
Collaborator

phybros commented May 29, 2023

v0.5.3 was just released. Enjoy!

@phybros phybros closed this as completed May 29, 2023
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

No branches or pull requests

4 participants