Skip to content

[fix](arrow-flight-sql) Fix Arrow Flight bearer token cache evict after expired #41754

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

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

xinyiZzz
Copy link
Contributor

Guava LoadingCache expiration check is performed when the cache is accessed, not immediately when the expiration time is reached.

So manually call the cleanUp method regularly to clean up expired cache items.

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@xinyiZzz
Copy link
Contributor Author

run buildall

@@ -82,6 +88,8 @@ public FlightTokenDetails load(String key) {
return new FlightTokenDetails();
}
});
this.cleanupExecutor = Executors.newScheduledThreadPool(1, new CustomThreadFactory("flight-token-cleanup"));
Copy link
Contributor

Choose a reason for hiding this comment

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

我们为什么不是再register connection的时候检查一下链接是否满了,主动淘汰呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不好,因为链接没满,也希望可以淘汰过期的token,

  1. 这里淘汰的是 flight token,也就是 arrow flight server 的登陆凭证,不是flight链接,这个 token 预期是要有过期淘汰机制的,类似账户登陆后一段时间就要重新登陆。
  2. 淘汰 token 会同时淘汰 flight 链接,因为如果 token 淘汰了那 flight 链接也就不会再访问了,用户 show processlist 看到 arrow flight 链接一直没有释放会有疑问。
  3. 默认 token 过期时间是 3天,flight/mysql 链接过期时间要更短,也就是一次登陆(拿到token)可以多次创建链接,所以不能在 flight 链接过期淘汰的同时淘汰 token。

Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Oct 16, 2024
Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link
Contributor

@wangbo wangbo left a comment

Choose a reason for hiding this comment

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

LGTM

@xinyiZzz xinyiZzz merged commit a74ee9a into apache:master Oct 16, 2024
28 of 32 checks passed
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request Oct 22, 2024
…er expired (apache#41754)

Guava LoadingCache expiration check is performed when the cache is
accessed, not immediately when the expiration time is reached.

So manually call the cleanUp method regularly to clean up expired cache
items.
yiguolei pushed a commit that referenced this pull request Oct 22, 2024
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request Nov 11, 2024
…er expired (apache#41754)

Guava LoadingCache expiration check is performed when the cache is
accessed, not immediately when the expiration time is reached.

So manually call the cleanUp method regularly to clean up expired cache
items.
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request Nov 18, 2024
…er expired (apache#41754)

Guava LoadingCache expiration check is performed when the cache is
accessed, not immediately when the expiration time is reached.

So manually call the cleanUp method regularly to clean up expired cache
items.
dataroaring pushed a commit that referenced this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.7-merged dev/3.0.3-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants