You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/09/07 13:43:15 UTC
[GitHub] [knox] smolnar82 opened a new pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
smolnar82 opened a new pull request #490:
URL: https://github.com/apache/knox/pull/490
## What changes were proposed in this pull request?
As described in the JIRA; in-memory lookup is skipped in JDBC token state service while fetching token expiration or metadata. Instead, we go directly to the DB in this implementation.
Since those queries are simple (there is no table join involved) and they rely on indexed columns the is no significant performance issue.
## How was this patch tested?
Updated JUnit test cases and repeated the same steps as described in the JIRA. With my fix, the disabled authentication request fails on node 2 like this:
```
$ curl -ku Passcode:WXpVNVlUZGlNRGt0WWpFMk5TMDBZamxsTFRobVpEY3ROV0psWW1WbE1EVTRZakF4OjpOVFptWW1VNVlXVXROelppTVMwME5URmhMVGcxWXpRdFl6Z3hNVEUwTmpkak5XUTA= https://localhost:8444/gateway/tokenbased/webhdfs/v1?op=LISTSTATUS
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
<title>Error 401 Token c59a7b09...5bebee058b01 is disabled</title>
</head>
<body><h2>HTTP ERROR 401 Token c59a7b09...5bebee058b01 is disabled</h2>
<table>
<tr><th>URI:</th><td>/gateway/tokenbased/webhdfs/v1</td></tr>
<tr><th>STATUS:</th><td>401</td></tr>
<tr><th>MESSAGE:</th><td>Token c59a7b09...5bebee058b01 is disabled</td></tr>
<tr><th>SERVLET:</th><td>tokenbased-knox-gateway-servlet</td></tr>
</table>
</body>
</html>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on pull request #490:
URL: https://github.com/apache/knox/pull/490#issuecomment-914318415
Cc. @zeroflag
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] zeroflag commented on pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
Posted by GitBox <gi...@apache.org>.
zeroflag commented on pull request #490:
URL: https://github.com/apache/knox/pull/490#issuecomment-914321239
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 merged pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
Posted by GitBox <gi...@apache.org>.
smolnar82 merged pull request #490:
URL: https://github.com/apache/knox/pull/490
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on pull request #490:
URL: https://github.com/apache/knox/pull/490#issuecomment-915837967
> > I'd the idea of replacing the current `ConcurrentHashMap` caches into `Caffeine.Cache` instances but we would still have an issue with eventual consistency within the configured entry TTL in those caches ("margin of error"). I discussed this approach with @zeroflag and we felt that it'd be pre-mature optimization that can be added later if we hit any performance issues. On the other hand, I'm open to re-evaluate that area if we insist this has to be done now.
>
> I see, I am cool with it knowing this has been discussed and will be addressed in the future patches :)
> Thanks!
https://issues.apache.org/jira/browse/KNOX-2660
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] moresandeep commented on pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
Posted by GitBox <gi...@apache.org>.
moresandeep commented on pull request #490:
URL: https://github.com/apache/knox/pull/490#issuecomment-915227354
> I'd the idea of replacing the current `ConcurrentHashMap` caches into `Caffeine.Cache` instances but we would still have an issue with eventual consistency within the configured entry TTL in those caches ("margin of error"). I discussed this approach with @zeroflag and we felt that it'd be pre-mature optimization that can be added later if we hit any performance issues. On the other hand, I'm open to re-evaluate that area if we insist this has to be done now.
I see, I am cool with it knowing this has been discussed and will be addressed in the future patches :)
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on pull request #490: KNOX-2658 - Skipping in-memory lookup while fetching expiration/metadata for a token
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on pull request #490:
URL: https://github.com/apache/knox/pull/490#issuecomment-915064438
Thanks, @moresandeep for the detailed comment. Let me try to answer your questions.
> Github does not let me click on line numbers that have not changed so adding my comments here. Why do we have some places that use in-memory implementation and some places that do not? it is a bit confusing.
I tried to keep the in-memory lookups where we fetch data that cannot be changed. So that it's gonna be the same on each node all the time (until the token is removed/expired).
> I think instead of removing the token completely can we adjust the TTL just for the metadata in case of JDBC to be short, say 3-5 mins and we can have that as a "margin of error". Or instead of removing the code, use a switch to figure out when HA is enabled so that non-HA JDBC implementations can still benefit from the cache performance. Thoughts?
I'd the idea of replacing the current `ConcurrentHashMap` caches into `Caffeine.Cache` instances but we would still have an issue with eventual consistency within the configured entry TTL in those caches ("margin of error"). I discussed this approach with @zeroflag and we felt that it'd be pre-mature optimization that can be added later if we hit any performance issues. On the other hand, I'm open to re-evaluate that area if we insist this has to be done now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org