-
Notifications
You must be signed in to change notification settings - Fork 322
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
feat: add a new implementation of schema with ttl #5462
base: master
Are you sure you want to change the base?
Conversation
6cc5479
to
ef40a95
Compare
1110046
to
f36f3e3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5462 +/- ##
==========================================
+ Coverage 74.88% 75.10% +0.21%
==========================================
Files 458 459 +1
Lines 63269 63424 +155
==========================================
+ Hits 47381 47632 +251
+ Misses 13235 13146 -89
+ Partials 2653 2646 -7 ☔ View full report in Codecov by Sentry. |
5cb6d75
to
a8081e0
Compare
warehouse/schema/schema_v2.go
Outdated
type schemaV2 struct { | ||
stats struct { | ||
schemaSize stats.Histogram | ||
} | ||
warehouse model.Warehouse | ||
log logger.Logger | ||
ttlInMinutes time.Duration | ||
schemaRepo schemaRepo | ||
stagingFilesSchemaPaginationSize int | ||
stagingFileRepo stagingFileRepo | ||
enableIDResolution bool | ||
fetchSchemaRepo fetchSchemaRepo | ||
now func() time.Time | ||
} |
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.
For all API calls are we excessively calling the database using sh.getSchema(ctx)
even for things like IsWarehouseSchemaEmpty
or GetTableSchemaInWarehouse
or GetColumnsCountInWarehouseSchema
? This will make too many DB calls.
Can't we store it within schemaV2
once and use it?
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.
Added 2 new fields in the struct cachedSchema
and cacheExpiry
to avoid DB calls
59fec54
to
fa7f166
Compare
62e7b27
to
fa7f166
Compare
fa7f166
to
28f7e95
Compare
28f7e95
to
7b96cdb
Compare
Description
This PR introduces schema invalidation based on a configurable TTL. When a schema entry expires, it is refreshed from the warehouse and updated in storage.
Changes:
expires_at
column to thewh_schemas
table.schema_v2
), gated by a feature flag.NewSchema
function to acceptwhManager
, allowing schema_v2 to reference the repository.Handler
interface to accept a context and return an error, as most schema_v2 methods now interact with the database.removeDeprecatedColumns
andtableSchemaDiff
from schemaV1 for reuse in schemaV2.Security