Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Remove dependence on Resource from FileCache #3431

Closed
wants to merge 1 commit into from

Conversation

jfirebaugh
Copy link
Contributor

Resource is going away as part of #3374.

The only place that Resource#kind was used was to bypass an attempt at compression for sprite images. But it's fine for that path to just do the compression and use it if it works -- even if not optimal, sprite images account for a relatively small portion of cache rows.

@kkaefer Does that sound ok? Also, is there a reason for http_cache_kind_idx? It doesn't seem like it would be utilized by any of the existing queries.

Resource is going away as part of #3374.

The only place that Resource#kind was used was to bypass an attempt
at compression for sprite images. But it's fine for that path to just do
the compression and use it if it works -- even if not optimal, sprite
images account for a relatively small portion of cache rows.
@kkaefer
Copy link
Member

kkaefer commented Jan 5, 2016

The idea of storing the kind was that we could discriminate in the cache based on type (e.g. not compress images when storing them in the cache). However, it's probably enough to do magic header detection for those specific use cases.

@@ -68,7 +68,7 @@ void SQLiteCache::Impl::createSchema() {
"CREATE TABLE IF NOT EXISTS `http_cache` ("
" `url` TEXT PRIMARY KEY NOT NULL,"
" `status` INTEGER NOT NULL," // The response status (Successful or Error).
" `kind` INTEGER NOT NULL," // The kind of file.
" `kind` INTEGER NOT NULL," // The kind of file. (No longer used; column remains only for backward compatibility.)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably remove this, as the cache will get recreated anyway after #3429. Don't forget to bump the cache schema version.

@jfirebaugh
Copy link
Contributor Author

Given recent offline discussions I'm not sure this is necessary.

@jfirebaugh jfirebaugh closed this Jan 5, 2016
@jfirebaugh jfirebaugh deleted the cache-url branch January 5, 2016 21:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants