Skip to content

Comments

refactor(dns): enhance cache safety, optimize performance, and refactor query logic#5248

Merged
RPRX merged 9 commits intoXTLS:mainfrom
Meo597:fix-p-dns-CacheCleanup
Nov 21, 2025
Merged

refactor(dns): enhance cache safety, optimize performance, and refactor query logic#5248
RPRX merged 9 commits intoXTLS:mainfrom
Meo597:fix-p-dns-CacheCleanup

Conversation

@Meo597
Copy link
Collaborator

@Meo597 Meo597 commented Oct 21, 2025 •
edited
Loading

此 PR 具体干了这些事：

  1. 给 dns cache 的 cleanup 糊了一个没什么大用的 shrink。开乐观缓存后 cache 膨胀或可节省几十 m 内存
  2. shrink 改为异步，开乐观缓存后 cache 膨胀、且大量缓存过期时防止这里成为阻塞点
  3. cleanup 锁太粗，改为先收集再写
  4. QueryIP 和 updateIP 设计有问题：代码冗余、锁太粗、需要二次锁了查缓存。全部重写
  5. findIPsForDomain 锁太粗，还混了 merge ip 的逻辑。重构了。现在 cache controller 只负责管理缓存
  6. 未开启缓存、缓存击穿时用 singleflight 保护
  7. 还有一些零碎的小 fix 记不清了，比如 record 更新时没有 copy 等安全问题等

PS:
现在先 publish(t1) 再 write cache(t2) 的机制，可能有极低概率导致 test...withcache 失败。
因为在 t1->t2 时隙 test 启动了第二次查询结果 miss cache。

其实没大问题因为时隙间所有请求都会被 singleflight 保护，最坏情况无非是迟迟无法命中缓存，导致每 rtt 都回源一次。
在目前 cache map 是全局锁的情况下，这已经是最优解了。之所以不让 test sleep 5s 单纯是想看看概率。

改动比较多，如果不好 review 可进一步拆成几个 PR

缓存重构(#5248)、乐观缓存(#5237)、并发查询(#5239)
三个 PR 相互冲突，如果全部接受得按这个顺序 merge
我好 rebase

@Fangliding
Copy link
Member

Fangliding commented Oct 22, 2025

?

@Meo597 Meo597 changed the title feat(dns): add cache cleanup shrink and rebuild logic to reclaim memory feat(dns): enhance cache safety, optimize performance, and refactor query logic Oct 24, 2025
@Meo597 Meo597 force-pushed the fix-p-dns-CacheCleanup branch from 8c0ea12 to 10d4562 Compare October 24, 2025 02:43
@Meo597

This comment was marked as outdated.

Sign in to view
@Meo597 Meo597 mentioned this pull request Oct 24, 2025
Merged
@t-e-s-tweb
Copy link

t-e-s-tweb commented Oct 24, 2025

This commit doesn't work correctly. After running for 5-10mins it stops resolving domains that were working before and just gives nxdomain errors e.g github.com .Please check.

@Meo597
Copy link
Collaborator Author

Meo597 commented Oct 24, 2025

@t-e-s-tweb 感谢测试！这是一个严重问题，我会在今天晚上再次 review 它

@Meo597
Copy link
Collaborator Author

Meo597 commented Oct 24, 2025 •
edited
Loading

@t-e-s-tweb 我已经修好了，测试后没发现问题，保险起见请你再测试一下

当缓存中同时存在 A + AAAA 且一个过期另一个没有时
如果请求已过期的，就会不报错地命中缓存并获得空 IP 集
然后上层调用者会将其转为 NXDOMAIN ERROR CODE 3

因为误认为已命中缓存因此永远不会刷新记录，直到 A + AAAA 同时过期后被 5 分钟一次的 cleanup 清理掉

这个 BUG 是我防御式编程引发的
在 review 时发现 rec.getIp() 不是调用者而是成员函数自己判定 nil
这不是一个好习惯，顺手改了一下引发灾难
这里业务逻辑太绕了

@Meo597

This comment was marked as resolved.

Sign in to view
@xqzr
Copy link
Contributor

xqzr commented Oct 24, 2025

xray 长期存在的一个小问题 如果缓存中有 A + AAAA 且其中一个已过期

请求双栈只能得到一个

6。怪不得我有时候失去 A 或 AAAA 结果，还以为是网络错误😇

@Meo597
Copy link
Collaborator Author

Meo597 commented Oct 25, 2025 •
edited
Loading

6。怪不得我有时候失去 A 或 AAAA 结果，还以为是网络错误😇

双栈请求，若 A + AAAA 缓存全部过期
会分两次各自回源
若 rtt > 先响应的 ttl
就会吞掉所有结果

如果一个过期另一个还剩一秒
就会续一秒，然后同上

@Meo597 Meo597 force-pushed the fix-p-dns-CacheCleanup branch from 26b70be to bba947d Compare October 25, 2025 21:05
This was referenced Oct 26, 2025
Merged
Merged
@Meo597 Meo597 changed the title feat(dns): enhance cache safety, optimize performance, and refactor query logic refactor(dns): enhance cache safety, optimize performance, and refactor query logic Oct 31, 2025
@RPRX
Copy link
Member

RPRX commented Nov 21, 2025

服了你们这些一天天不断折腾 DNS 代码的，合！都可以合！反正出问题了你们修

@RPRX RPRX merged commit b40bf56 into XTLS:main Nov 21, 2025
81 of 82 checks passed
@Meo597
Copy link
Collaborator Author

Meo597 commented Nov 21, 2025

DNS 的三个 PR 在我这 UDP + TPROXY 环境下跑了快一个月了
TCP QUIC 啥的他们也测了
应该没人比我分流规则更复杂了
包没问题的

@Meo597 Meo597 deleted the fix-p-dns-CacheCleanup branch November 21, 2025 07:22
This was referenced Dec 1, 2025
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yuhan6665 yuhan6665 Awaiting requested review from yuhan6665

@Fangliding Fangliding Awaiting requested review from Fangliding

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Meo597 @Fangliding @t-e-s-tweb @xqzr @RPRX