servertap-io/servertap

Webhook timings issue - Paper 1.17.1

c1oneman opened this issue · 19 comments

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

@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

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

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

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' 

Can you try again with v0.5.0 @LittleChest

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.

Can you try again with v0.5.0 @LittleChest

did not solve the problem
image

MeesJ commented

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

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

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

MeesJ commented

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

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

I'll see if I can send a build shortly

MeesJ commented

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.

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. ฅ⁠^⁠•⁠ﻌ⁠•⁠^⁠ฅ

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 commented

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

MeesJ commented

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.

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

v0.5.3 was just released. Enjoy!