-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add Table metadata cache expiration configuration #321
Add Table metadata cache expiration configuration #321
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I left one comment since I'm not sure about the use case.
CacheBuilder<Object, Object> builder = CacheBuilder.newBuilder(); | ||
long tableMetadataCacheExpirationTimeSecs = config.getTableMetadataCacheExpirationTimeSecs(); | ||
if (tableMetadataCacheExpirationTimeSecs > 0) { | ||
builder.expireAfterWrite(tableMetadataCacheExpirationTimeSecs, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure about the use case of specifying the expiration time. Can you clarify?
From users' perspective, it seems like it should be cached forever or it should be replaced right after it needs to
be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case is when the table metadata change happens. As we don't have alter table
feature for now, create table
and delete table
are only the use cases. Anyway, I think we will need to handle alter table
cases in the future, so this cache expiration feature will become more important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
But the CIs are failing. Can you take a look?
They were successful after rerunning the CIs. I'm merging this. |
This PR allows us to set Table metadata cache expiration configuration with the
scalar.db.table_metadata.cache_expiration_time_secs
property. Please take a look!