Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-11-25 Thread Aaron Lipman
Thanks for the reminder, Richard.

Wanted to ping folks that I've submitted a PR adding first and last IDs to
cache versions:
https://github.com/rails/rails/pull/37724

(My reasoning for using bounding IDs over a checksum is contained in the
summary, but I wanted to restate my gratitude for others' input.)

I greatly appreciate further review!



On Tue, Nov 5, 2019 at 11:38 AM richard schneeman <
richard.schnee...@gmail.com> wrote:

> Love this passion and energy. Can you all move the conversation over to
> the issue so that it's easier for other people have a similar problem to
> find or chime in?
>
> On Tue, Nov 5, 2019 at 8:16 AM Aaron Lipman  wrote:
>
>> Again, good point. There are some workarounds (
>> https://evrim.io/invalidating-caches-when-using-many-to-many-associations-in-rails/),
>> but the absence of a touch: true option on has_many relations makes the
>> last ID approach less optimal.
>>
>> Also, I'm noting that support for the LAST_VERSION function didn't get
>> added to SQLite3 until version 3.25.0 (2018). As much as I liked the idea
>> of the last ID approach, I'm not seeing a clean way to implement that
>> doesn't require Rails devs to update their development environment.
>>
>> I'm going to continue to pursue a hash-based approach. Thanks again for
>> your insights, Daniel!
>>
>> On Mon, Nov 4, 2019 at 5:58 PM Daniel  wrote:
>>
>>> The last ID isn't stable if you replace an item within the collection
>>> though, right (assuming it's via a HABTM, so the associated record doesn't
>>> get touched)?
>>>
>>>
>>> On 11/5/19 2:21 AM, Aaron Lipman wrote:
>>>
>>> Been thinking on this a few days, here are a couple potential solutions
>>> I've been considering:
>>>
>>> Hash-based: Sum the MD5 hashes of the IDs and append the last 32
>>> hexadecimal digits of the sum to the cache version. Alternatively, use a
>>> mathematical hashing function like Fibonacci Hashing
>>> .
>>> (The second approach wouldn't accommodate non-numeric ID columns, but may
>>> be easier to implement across various SQL flavors without any conditional
>>> logic.) Both approaches need to account for the various 64-bit limits found
>>> in MySQL and Postgres, e.g. MySQL's CONV() function.
>>>
>>> Last ID based: It occurs to me that when a record within a collection is
>>> destroyed, either the size of the collection or the ID of its last item
>>> will change. If we can reliably get that last ID, we can use it in
>>> conjunction with collection size to generate more robust cache keys. The
>>> LAST_VALUE sql function might provide this. However, MySQL didn't add
>>> LAST_VALUE until version 8 (2018), but I think we can emulate the same
>>> functionality via MySQL session variables.
>>>
>>> I'm leaning towards the "Last ID" path. I think it's a little more
>>> elegant and probably more efficient than calculating a sum when it comes to
>>> really large collections. Starting work on a pull request, but welcome
>>> other ideas/directions/considerations.
>>>
>>>
>>> On Wed, Oct 30, 2019 at 6:19 PM Daniel Heath 
>>>  wrote:
>>>
 I think it’s worth considering  implementing per database, as the major
 ones all have something that’ll work and the keys don’t need to be stable
 across different data store implementations.

 Thanks,
 Daniel Heath

 On 31 Oct 2019, at 6:17 am, Aaron Lipman  wrote:

 
 Thanks for the flag, Daniel. On my first read of the code, I didn't
 catch that an aggregate query was used to determine the max updated_at
 timestamp (without fetching individual records) if a relation wasn't
 loaded.

 Figuring out a database-side hashing function that works with all the
 flavors of SQL supported by Rails may prove tricky. I'm going to consider
 this some more, but an alternative approach might be to update the
 documentation to acknowledge this issue and explain how to customize a
 model's cache_version method according to one's needs.

 On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath 
  wrote:

> The full collection could be millions of records. Fetching the ids to
> hash might not be an option either, unless they can be hashed on the
> database side.
>
> Thanks,
> Daniel Heath
>
> On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:
>
> 
> Hi Marc & Richard,
>
> I'd categorize this as a bug. When generating a cache_version for a
> collection, one solution might be to include a hash of the IDs of all 
> items
> in the collection. This way, the cache_version will change should an item
> be removed.
>
> I believe modifying the compute_cache_version method defined in
> activerecord/lib/active_record/relation.rb
> 
> is the way 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-11-05 Thread richard schneeman
Love this passion and energy. Can you all move the conversation over to the
issue so that it's easier for other people have a similar problem to find
or chime in?

On Tue, Nov 5, 2019 at 8:16 AM Aaron Lipman  wrote:

> Again, good point. There are some workarounds (
> https://evrim.io/invalidating-caches-when-using-many-to-many-associations-in-rails/),
> but the absence of a touch: true option on has_many relations makes the
> last ID approach less optimal.
>
> Also, I'm noting that support for the LAST_VERSION function didn't get
> added to SQLite3 until version 3.25.0 (2018). As much as I liked the idea
> of the last ID approach, I'm not seeing a clean way to implement that
> doesn't require Rails devs to update their development environment.
>
> I'm going to continue to pursue a hash-based approach. Thanks again for
> your insights, Daniel!
>
> On Mon, Nov 4, 2019 at 5:58 PM Daniel  wrote:
>
>> The last ID isn't stable if you replace an item within the collection
>> though, right (assuming it's via a HABTM, so the associated record doesn't
>> get touched)?
>>
>>
>> On 11/5/19 2:21 AM, Aaron Lipman wrote:
>>
>> Been thinking on this a few days, here are a couple potential solutions
>> I've been considering:
>>
>> Hash-based: Sum the MD5 hashes of the IDs and append the last 32
>> hexadecimal digits of the sum to the cache version. Alternatively, use a
>> mathematical hashing function like Fibonacci Hashing
>> .
>> (The second approach wouldn't accommodate non-numeric ID columns, but may
>> be easier to implement across various SQL flavors without any conditional
>> logic.) Both approaches need to account for the various 64-bit limits found
>> in MySQL and Postgres, e.g. MySQL's CONV() function.
>>
>> Last ID based: It occurs to me that when a record within a collection is
>> destroyed, either the size of the collection or the ID of its last item
>> will change. If we can reliably get that last ID, we can use it in
>> conjunction with collection size to generate more robust cache keys. The
>> LAST_VALUE sql function might provide this. However, MySQL didn't add
>> LAST_VALUE until version 8 (2018), but I think we can emulate the same
>> functionality via MySQL session variables.
>>
>> I'm leaning towards the "Last ID" path. I think it's a little more
>> elegant and probably more efficient than calculating a sum when it comes to
>> really large collections. Starting work on a pull request, but welcome
>> other ideas/directions/considerations.
>>
>>
>> On Wed, Oct 30, 2019 at 6:19 PM Daniel Heath 
>>  wrote:
>>
>>> I think it’s worth considering  implementing per database, as the major
>>> ones all have something that’ll work and the keys don’t need to be stable
>>> across different data store implementations.
>>>
>>> Thanks,
>>> Daniel Heath
>>>
>>> On 31 Oct 2019, at 6:17 am, Aaron Lipman  wrote:
>>>
>>> 
>>> Thanks for the flag, Daniel. On my first read of the code, I didn't
>>> catch that an aggregate query was used to determine the max updated_at
>>> timestamp (without fetching individual records) if a relation wasn't
>>> loaded.
>>>
>>> Figuring out a database-side hashing function that works with all the
>>> flavors of SQL supported by Rails may prove tricky. I'm going to consider
>>> this some more, but an alternative approach might be to update the
>>> documentation to acknowledge this issue and explain how to customize a
>>> model's cache_version method according to one's needs.
>>>
>>> On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath 
>>>  wrote:
>>>
 The full collection could be millions of records. Fetching the ids to
 hash might not be an option either, unless they can be hashed on the
 database side.

 Thanks,
 Daniel Heath

 On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:

 
 Hi Marc & Richard,

 I'd categorize this as a bug. When generating a cache_version for a
 collection, one solution might be to include a hash of the IDs of all items
 in the collection. This way, the cache_version will change should an item
 be removed.

 I believe modifying the compute_cache_version method defined in
 activerecord/lib/active_record/relation.rb
 
 is the way to go about this.

 Marc, please let me know if you intend to submit a pull request. While
 I'm not on the Rails team, I'd be happy to offer any assistance and lobby
 for implementing a fix. (If you're not interested in submitting a PR, I'd
 be excited to pick this up myself.)

 On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge <
 marckohlbru...@gmail.com> wrote:

>  https://github.com/rails/rails/issues/37555
>
> I did find a few similar issues, but they all got marked as stale
>
> 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-11-05 Thread Aaron Lipman
Again, good point. There are some workarounds (
https://evrim.io/invalidating-caches-when-using-many-to-many-associations-in-rails/),
but the absence of a touch: true option on has_many relations makes the
last ID approach less optimal.

Also, I'm noting that support for the LAST_VERSION function didn't get
added to SQLite3 until version 3.25.0 (2018). As much as I liked the idea
of the last ID approach, I'm not seeing a clean way to implement that
doesn't require Rails devs to update their development environment.

I'm going to continue to pursue a hash-based approach. Thanks again for
your insights, Daniel!

On Mon, Nov 4, 2019 at 5:58 PM Daniel  wrote:

> The last ID isn't stable if you replace an item within the collection
> though, right (assuming it's via a HABTM, so the associated record doesn't
> get touched)?
>
>
> On 11/5/19 2:21 AM, Aaron Lipman wrote:
>
> Been thinking on this a few days, here are a couple potential solutions
> I've been considering:
>
> Hash-based: Sum the MD5 hashes of the IDs and append the last 32
> hexadecimal digits of the sum to the cache version. Alternatively, use a
> mathematical hashing function like Fibonacci Hashing
> .
> (The second approach wouldn't accommodate non-numeric ID columns, but may
> be easier to implement across various SQL flavors without any conditional
> logic.) Both approaches need to account for the various 64-bit limits found
> in MySQL and Postgres, e.g. MySQL's CONV() function.
>
> Last ID based: It occurs to me that when a record within a collection is
> destroyed, either the size of the collection or the ID of its last item
> will change. If we can reliably get that last ID, we can use it in
> conjunction with collection size to generate more robust cache keys. The
> LAST_VALUE sql function might provide this. However, MySQL didn't add
> LAST_VALUE until version 8 (2018), but I think we can emulate the same
> functionality via MySQL session variables.
>
> I'm leaning towards the "Last ID" path. I think it's a little more elegant
> and probably more efficient than calculating a sum when it comes to really
> large collections. Starting work on a pull request, but welcome other
> ideas/directions/considerations.
>
>
> On Wed, Oct 30, 2019 at 6:19 PM Daniel Heath 
>  wrote:
>
>> I think it’s worth considering  implementing per database, as the major
>> ones all have something that’ll work and the keys don’t need to be stable
>> across different data store implementations.
>>
>> Thanks,
>> Daniel Heath
>>
>> On 31 Oct 2019, at 6:17 am, Aaron Lipman  wrote:
>>
>> 
>> Thanks for the flag, Daniel. On my first read of the code, I didn't catch
>> that an aggregate query was used to determine the max updated_at timestamp
>> (without fetching individual records) if a relation wasn't loaded.
>>
>> Figuring out a database-side hashing function that works with all the
>> flavors of SQL supported by Rails may prove tricky. I'm going to consider
>> this some more, but an alternative approach might be to update the
>> documentation to acknowledge this issue and explain how to customize a
>> model's cache_version method according to one's needs.
>>
>> On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath 
>>  wrote:
>>
>>> The full collection could be millions of records. Fetching the ids to
>>> hash might not be an option either, unless they can be hashed on the
>>> database side.
>>>
>>> Thanks,
>>> Daniel Heath
>>>
>>> On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:
>>>
>>> 
>>> Hi Marc & Richard,
>>>
>>> I'd categorize this as a bug. When generating a cache_version for a
>>> collection, one solution might be to include a hash of the IDs of all items
>>> in the collection. This way, the cache_version will change should an item
>>> be removed.
>>>
>>> I believe modifying the compute_cache_version method defined in
>>> activerecord/lib/active_record/relation.rb
>>> 
>>> is the way to go about this.
>>>
>>> Marc, please let me know if you intend to submit a pull request. While
>>> I'm not on the Rails team, I'd be happy to offer any assistance and lobby
>>> for implementing a fix. (If you're not interested in submitting a PR, I'd
>>> be excited to pick this up myself.)
>>>
>>> On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge <
>>> marckohlbru...@gmail.com> wrote:
>>>
  https://github.com/rails/rails/issues/37555

 I did find a few similar issues, but they all got marked as stale

 https://github.com/rails/rails/issues/34408
 https://github.com/rails/rails/issues/31996
 https://github.com/rails/rails/issues/34093

 I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm
 not supposed to fragment cache my paginated results. Using collection
 caching (`render collection: @posts, cached: 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-11-04 Thread Daniel
The last ID isn't stable if you replace an item within the collection 
though, right (assuming it's via a HABTM, so the associated record 
doesn't get touched)?



On 11/5/19 2:21 AM, Aaron Lipman wrote:
Been thinking on this a few days, here are a couple potential 
solutions I've been considering:


Hash-based: Sum the MD5 hashes of the IDs and append the last 32 
hexadecimal digits of the sum to the cache version. Alternatively, use 
a mathematical hashing function like Fibonacci Hashing 
. 
(The second approach wouldn't accommodate non-numeric ID columns, but 
may be easier to implement across various SQL flavors without any 
conditional logic.) Both approaches need to account for the various 
64-bit limits found in MySQL and Postgres, e.g. MySQL's CONV() function.


Last ID based: It occurs to me that when a record within a collection 
is destroyed, either the size of the collection or the ID of its last 
item will change. If we can reliably get that last ID, we can use it 
in conjunction with collection size to generate more robust cache 
keys. The LAST_VALUE sql function might provide this. However, MySQL 
didn't add LAST_VALUE until version 8 (2018), but I think we can 
emulate the same functionality via MySQL session variables.


I'm leaning towards the "Last ID" path. I think it's a little more 
elegant and probably more efficient than calculating a sum when it 
comes to really large collections. Starting work on a pull request, 
but welcome other ideas/directions/considerations.



On Wed, Oct 30, 2019 at 6:19 PM Daniel Heath  wrote:

I think it’s worth considering  implementing per database, as the
major ones all have something that’ll work and the keys don’t need
to be stable across different data store implementations.

Thanks,
Daniel Heath


On 31 Oct 2019, at 6:17 am, Aaron Lipman mailto:alipma...@gmail.com>> wrote:


Thanks for the flag, Daniel. On my first read of the code, I
didn't catch that an aggregate query was used to determine the
max updated_at timestamp (without fetching individual records) if
a relation wasn't loaded.

Figuring out a database-side hashing function that works with all
the flavors of SQL supported by Rails may prove tricky. I'm going
to consider this some more, but an alternative approach might be
to update the documentation to acknowledge this issue and explain
how to customize a model's cache_version method according to
one's needs.

On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath  wrote:

The full collection could be millions of records. Fetching
the ids to hash might not be an option either, unless they
can be hashed on the database side.

Thanks,
Daniel Heath


On 29 Oct 2019, at 4:22 am, Aaron Lipman
mailto:alipma...@gmail.com>> wrote:


Hi Marc & Richard,

I'd categorize this as a bug. When generating a
cache_version for a collection, one solution might be to
include a hash of the IDs of all items in the collection.
This way, the cache_version will change should an item be
removed.

I believe modifying the compute_cache_version method defined
in activerecord/lib/active_record/relation.rb


is the way to go about this.

Marc, please let me know if you intend to submit a pull
request. While I'm not on the Rails team, I'd be happy to
offer any assistance and lobby for implementing a fix. (If
you're not interested in submitting a PR, I'd be excited to
pick this up myself.)

On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge
mailto:marckohlbru...@gmail.com>>
wrote:

 https://github.com/rails/rails/issues/37555

I did find a few similar issues, but they all got marked
as stale

https://github.com/rails/rails/issues/34408
https://github.com/rails/rails/issues/31996
https://github.com/rails/rails/issues/34093

I'm surprised this doesn't appear to be a bigger
problem. Perhaps I'm not supposed to fragment cache my
paginated results. Using collection caching (`render
collection: @posts, cached: true`) might be performant
enough.

On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard
schneeman wrote:

Sounds like a bug. Can you move into an issue?



On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge
 wrote:

I'm running into a problem when using fragment
caching with active record collections.

Rails 6 uses a 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-11-04 Thread Aaron Lipman
Been thinking on this a few days, here are a couple potential solutions
I've been considering:

Hash-based: Sum the MD5 hashes of the IDs and append the last 32
hexadecimal digits of the sum to the cache version. Alternatively, use a
mathematical hashing function like Fibonacci Hashing
.
(The second approach wouldn't accommodate non-numeric ID columns, but may
be easier to implement across various SQL flavors without any conditional
logic.) Both approaches need to account for the various 64-bit limits found
in MySQL and Postgres, e.g. MySQL's CONV() function.

Last ID based: It occurs to me that when a record within a collection is
destroyed, either the size of the collection or the ID of its last item
will change. If we can reliably get that last ID, we can use it in
conjunction with collection size to generate more robust cache keys. The
LAST_VALUE sql function might provide this. However, MySQL didn't add
LAST_VALUE until version 8 (2018), but I think we can emulate the same
functionality via MySQL session variables.

I'm leaning towards the "Last ID" path. I think it's a little more elegant
and probably more efficient than calculating a sum when it comes to really
large collections. Starting work on a pull request, but welcome other
ideas/directions/considerations.


On Wed, Oct 30, 2019 at 6:19 PM Daniel Heath  wrote:

> I think it’s worth considering  implementing per database, as the major
> ones all have something that’ll work and the keys don’t need to be stable
> across different data store implementations.
>
> Thanks,
> Daniel Heath
>
> On 31 Oct 2019, at 6:17 am, Aaron Lipman  wrote:
>
> 
> Thanks for the flag, Daniel. On my first read of the code, I didn't catch
> that an aggregate query was used to determine the max updated_at timestamp
> (without fetching individual records) if a relation wasn't loaded.
>
> Figuring out a database-side hashing function that works with all the
> flavors of SQL supported by Rails may prove tricky. I'm going to consider
> this some more, but an alternative approach might be to update the
> documentation to acknowledge this issue and explain how to customize a
> model's cache_version method according to one's needs.
>
> On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath  wrote:
>
>> The full collection could be millions of records. Fetching the ids to
>> hash might not be an option either, unless they can be hashed on the
>> database side.
>>
>> Thanks,
>> Daniel Heath
>>
>> On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:
>>
>> 
>> Hi Marc & Richard,
>>
>> I'd categorize this as a bug. When generating a cache_version for a
>> collection, one solution might be to include a hash of the IDs of all items
>> in the collection. This way, the cache_version will change should an item
>> be removed.
>>
>> I believe modifying the compute_cache_version method defined in
>> activerecord/lib/active_record/relation.rb
>> 
>> is the way to go about this.
>>
>> Marc, please let me know if you intend to submit a pull request. While
>> I'm not on the Rails team, I'd be happy to offer any assistance and lobby
>> for implementing a fix. (If you're not interested in submitting a PR, I'd
>> be excited to pick this up myself.)
>>
>> On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge <
>> marckohlbru...@gmail.com> wrote:
>>
>>>  https://github.com/rails/rails/issues/37555
>>>
>>> I did find a few similar issues, but they all got marked as stale
>>>
>>> https://github.com/rails/rails/issues/34408
>>> https://github.com/rails/rails/issues/31996
>>> https://github.com/rails/rails/issues/34093
>>>
>>> I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm
>>> not supposed to fragment cache my paginated results. Using collection
>>> caching (`render collection: @posts, cached: true`) might be performant
>>> enough.
>>>
>>> On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard schneeman wrote:

 Sounds like a bug. Can you move into an issue?



 On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge <
 h...@marckohlbrugge.com> wrote:

> I'm running into a problem when using fragment caching with active
> record collections.
>
> Rails 6 uses a combination of collection.cache_key and
> collection.cache_version to determine whether a fragment cache is fresh or
> not. However, there is a scenario where the collection might change while
> collection.cache_key and collection.cache_version stay the same.
>
> It happens when you use a limit on the collection and then destroy one
> of those records, as long as it's not the most recent one. This way
> collection.cache_key will stay the same, because the query does not 
> change.
> And collection.cache_version will stay the same as well, 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-30 Thread Daniel Heath
I think it’s worth considering  implementing per database, as the major ones 
all have something that’ll work and the keys don’t need to be stable across 
different data store implementations. 

Thanks,
Daniel Heath 

> On 31 Oct 2019, at 6:17 am, Aaron Lipman  wrote:
> 
> 
> Thanks for the flag, Daniel. On my first read of the code, I didn't catch 
> that an aggregate query was used to determine the max updated_at timestamp 
> (without fetching individual records) if a relation wasn't loaded.
> 
> Figuring out a database-side hashing function that works with all the flavors 
> of SQL supported by Rails may prove tricky. I'm going to consider this some 
> more, but an alternative approach might be to update the documentation to 
> acknowledge this issue and explain how to customize a model's cache_version 
> method according to one's needs.
> 
>> On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath  wrote:
>> The full collection could be millions of records. Fetching the ids to hash 
>> might not be an option either, unless they can be hashed on the database 
>> side.
>> 
>> Thanks,
>> Daniel Heath 
>> 
 On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:
 
>>> 
>>> Hi Marc & Richard,
>>> 
>>> I'd categorize this as a bug. When generating a cache_version for a 
>>> collection, one solution might be to include a hash of the IDs of all items 
>>> in the collection. This way, the cache_version will change should an item 
>>> be removed.
>>> 
>>> I believe modifying the compute_cache_version method defined in 
>>> activerecord/lib/active_record/relation.rb is the way to go about this.
>>> 
>>> Marc, please let me know if you intend to submit a pull request. While I'm 
>>> not on the Rails team, I'd be happy to offer any assistance and lobby for 
>>> implementing a fix. (If you're not interested in submitting a PR, I'd be 
>>> excited to pick this up myself.)
>>> 
 On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge 
  wrote:
  https://github.com/rails/rails/issues/37555
 
 I did find a few similar issues, but they all got marked as stale
 
 https://github.com/rails/rails/issues/34408
 https://github.com/rails/rails/issues/31996
 https://github.com/rails/rails/issues/34093
 
 I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm not 
 supposed to fragment cache my paginated results. Using collection caching 
 (`render collection: @posts, cached: true`) might be performant enough.
 
> On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard schneeman wrote:
> Sounds like a bug. Can you move into an issue? 
> 
> 
> 
>> On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge 
>>  wrote:
>> I'm running into a problem when using fragment caching with active 
>> record collections.
>> 
>> Rails 6 uses a combination of collection.cache_key and 
>> collection.cache_version to determine whether a fragment cache is fresh 
>> or not. However, there is a scenario where the collection might change 
>> while collection.cache_key and collection.cache_version stay the same.
>> 
>> It happens when you use a limit on the collection and then destroy one 
>> of those records, as long as it's not the most recent one. This way 
>> collection.cache_key will stay the same, because the query does not 
>> change. And collection.cache_version will stay the same as well, because 
>> the collection count will remain unchanged as does the maximum 
>> updated_at timestamp of the most recent record.
>> 
>> I've build a sample app for demonstration purposes:
>> 
>> https://github.com/marckohlbrugge/rails-6-collection-caching-bug
>> 
>> The readme describes a way to reproduce the issue via the website 
>> itself, or the Rails console.
>> 
>> Would this be considered a bug or expected behavior? Are there any known 
>> work arounds?
>> -- 
>> You received this message because you are subscribed to the Google 
>> Groups "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send 
>> an email to rubyonra...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com.
> -- 
> Richard Schneeman
> https://www.schneems.com
 
 -- 
 You received this message because you are subscribed to the Google Groups 
 "Ruby on Rails: Core" group.
 To unsubscribe from this group and stop receiving emails from it, send an 
 email to rubyonrails-core+unsubscr...@googlegroups.com.
 To view this discussion on the web visit 
 https://groups.google.com/d/msgid/rubyonrails-core/6590103e-3528-4896-bc07-e6ff6a194bef%40googlegroups.com.
>>> 
>>> -- 
>>> You received this message because you are subscribed to the Google Groups 
>>> "Ruby on Rails: Core" group.
>>> To 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-30 Thread Aaron Lipman
Thanks for the flag, Daniel. On my first read of the code, I didn't catch
that an aggregate query was used to determine the max updated_at timestamp
(without fetching individual records) if a relation wasn't loaded.

Figuring out a database-side hashing function that works with all the
flavors of SQL supported by Rails may prove tricky. I'm going to consider
this some more, but an alternative approach might be to update the
documentation to acknowledge this issue and explain how to customize a
model's cache_version method according to one's needs.

On Mon, Oct 28, 2019 at 4:31 PM Daniel Heath  wrote:

> The full collection could be millions of records. Fetching the ids to hash
> might not be an option either, unless they can be hashed on the database
> side.
>
> Thanks,
> Daniel Heath
>
> On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:
>
> 
> Hi Marc & Richard,
>
> I'd categorize this as a bug. When generating a cache_version for a
> collection, one solution might be to include a hash of the IDs of all items
> in the collection. This way, the cache_version will change should an item
> be removed.
>
> I believe modifying the compute_cache_version method defined in
> activerecord/lib/active_record/relation.rb
> 
> is the way to go about this.
>
> Marc, please let me know if you intend to submit a pull request. While I'm
> not on the Rails team, I'd be happy to offer any assistance and lobby for
> implementing a fix. (If you're not interested in submitting a PR, I'd be
> excited to pick this up myself.)
>
> On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge 
> wrote:
>
>>  https://github.com/rails/rails/issues/37555
>>
>> I did find a few similar issues, but they all got marked as stale
>>
>> https://github.com/rails/rails/issues/34408
>> https://github.com/rails/rails/issues/31996
>> https://github.com/rails/rails/issues/34093
>>
>> I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm not
>> supposed to fragment cache my paginated results. Using collection caching
>> (`render collection: @posts, cached: true`) might be performant enough.
>>
>> On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard schneeman wrote:
>>>
>>> Sounds like a bug. Can you move into an issue?
>>>
>>>
>>>
>>> On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge 
>>> wrote:
>>>
 I'm running into a problem when using fragment caching with active
 record collections.

 Rails 6 uses a combination of collection.cache_key and
 collection.cache_version to determine whether a fragment cache is fresh or
 not. However, there is a scenario where the collection might change while
 collection.cache_key and collection.cache_version stay the same.

 It happens when you use a limit on the collection and then destroy one
 of those records, as long as it's not the most recent one. This way
 collection.cache_key will stay the same, because the query does not change.
 And collection.cache_version will stay the same as well, because the
 collection count will remain unchanged as does the maximum updated_at
 timestamp of the most recent record.

 I've build a sample app for demonstration purposes:

 https://github.com/marckohlbrugge/rails-6-collection-caching-bug

 The readme describes a way to reproduce the issue via the website
 itself, or the Rails console.

 Would this be considered a bug or expected behavior? Are there any
 known work arounds?

 --
 You received this message because you are subscribed to the Google
 Groups "Ruby on Rails: Core" group.
 To unsubscribe from this group and stop receiving emails from it, send
 an email to rubyonra...@googlegroups.com.
 To view this discussion on the web visit
 https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com
 
 .

>>> --
>>> Richard Schneeman
>>> https://www.schneems.com
>>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to rubyonrails-core+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/rubyonrails-core/6590103e-3528-4896-bc07-e6ff6a194bef%40googlegroups.com
>> 
>> .
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To view 

Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-28 Thread Daniel Heath
The full collection could be millions of records. Fetching the ids to hash 
might not be an option either, unless they can be hashed on the database side.

Thanks,
Daniel Heath 

> On 29 Oct 2019, at 4:22 am, Aaron Lipman  wrote:
> 
> 
> Hi Marc & Richard,
> 
> I'd categorize this as a bug. When generating a cache_version for a 
> collection, one solution might be to include a hash of the IDs of all items 
> in the collection. This way, the cache_version will change should an item be 
> removed.
> 
> I believe modifying the compute_cache_version method defined in 
> activerecord/lib/active_record/relation.rb is the way to go about this.
> 
> Marc, please let me know if you intend to submit a pull request. While I'm 
> not on the Rails team, I'd be happy to offer any assistance and lobby for 
> implementing a fix. (If you're not interested in submitting a PR, I'd be 
> excited to pick this up myself.)
> 
>> On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge  
>> wrote:
>>  https://github.com/rails/rails/issues/37555
>> 
>> I did find a few similar issues, but they all got marked as stale
>> 
>> https://github.com/rails/rails/issues/34408
>> https://github.com/rails/rails/issues/31996
>> https://github.com/rails/rails/issues/34093
>> 
>> I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm not 
>> supposed to fragment cache my paginated results. Using collection caching 
>> (`render collection: @posts, cached: true`) might be performant enough.
>> 
>>> On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard schneeman wrote:
>>> Sounds like a bug. Can you move into an issue? 
>>> 
>>> 
>>> 
 On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge  
 wrote:
 I'm running into a problem when using fragment caching with active record 
 collections.
 
 Rails 6 uses a combination of collection.cache_key and 
 collection.cache_version to determine whether a fragment cache is fresh or 
 not. However, there is a scenario where the collection might change while 
 collection.cache_key and collection.cache_version stay the same.
 
 It happens when you use a limit on the collection and then destroy one of 
 those records, as long as it's not the most recent one. This way 
 collection.cache_key will stay the same, because the query does not 
 change. And collection.cache_version will stay the same as well, because 
 the collection count will remain unchanged as does the maximum updated_at 
 timestamp of the most recent record.
 
 I've build a sample app for demonstration purposes:
 
 https://github.com/marckohlbrugge/rails-6-collection-caching-bug
 
 The readme describes a way to reproduce the issue via the website itself, 
 or the Rails console.
 
 Would this be considered a bug or expected behavior? Are there any known 
 work arounds?
 -- 
 You received this message because you are subscribed to the Google Groups 
 "Ruby on Rails: Core" group.
 To unsubscribe from this group and stop receiving emails from it, send an 
 email to rubyonra...@googlegroups.com.
 To view this discussion on the web visit 
 https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com.
>>> -- 
>>> Richard Schneeman
>>> https://www.schneems.com
>> 
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-core+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/rubyonrails-core/6590103e-3528-4896-bc07-e6ff6a194bef%40googlegroups.com.
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/rubyonrails-core/CAEJZ43iCipyOZddZAT1WCrtQ7g7VRLeokgLRGcYroK-TvGoGWw%40mail.gmail.com.

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/rubyonrails-core/4A85F762-275C-4796-9A47-0586833DADFA%40heath.cc.


Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-28 Thread Aaron Lipman
Hi Marc & Richard,

I'd categorize this as a bug. When generating a cache_version for a
collection, one solution might be to include a hash of the IDs of all items
in the collection. This way, the cache_version will change should an item
be removed.

I believe modifying the compute_cache_version method defined in
activerecord/lib/active_record/relation.rb

is the way to go about this.

Marc, please let me know if you intend to submit a pull request. While I'm
not on the Rails team, I'd be happy to offer any assistance and lobby for
implementing a fix. (If you're not interested in submitting a PR, I'd be
excited to pick this up myself.)

On Sun, Oct 27, 2019 at 10:41 AM Marc Köhlbrugge 
wrote:

>  https://github.com/rails/rails/issues/37555
>
> I did find a few similar issues, but they all got marked as stale
>
> https://github.com/rails/rails/issues/34408
> https://github.com/rails/rails/issues/31996
> https://github.com/rails/rails/issues/34093
>
> I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm not
> supposed to fragment cache my paginated results. Using collection caching
> (`render collection: @posts, cached: true`) might be performant enough.
>
> On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard schneeman wrote:
>>
>> Sounds like a bug. Can you move into an issue?
>>
>>
>>
>> On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge 
>> wrote:
>>
>>> I'm running into a problem when using fragment caching with active
>>> record collections.
>>>
>>> Rails 6 uses a combination of collection.cache_key and
>>> collection.cache_version to determine whether a fragment cache is fresh or
>>> not. However, there is a scenario where the collection might change while
>>> collection.cache_key and collection.cache_version stay the same.
>>>
>>> It happens when you use a limit on the collection and then destroy one
>>> of those records, as long as it's not the most recent one. This way
>>> collection.cache_key will stay the same, because the query does not change.
>>> And collection.cache_version will stay the same as well, because the
>>> collection count will remain unchanged as does the maximum updated_at
>>> timestamp of the most recent record.
>>>
>>> I've build a sample app for demonstration purposes:
>>>
>>> https://github.com/marckohlbrugge/rails-6-collection-caching-bug
>>>
>>> The readme describes a way to reproduce the issue via the website
>>> itself, or the Rails console.
>>>
>>> Would this be considered a bug or expected behavior? Are there any known
>>> work arounds?
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Ruby on Rails: Core" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to rubyonra...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com
>>> 
>>> .
>>>
>> --
>> Richard Schneeman
>> https://www.schneems.com
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/rubyonrails-core/6590103e-3528-4896-bc07-e6ff6a194bef%40googlegroups.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/rubyonrails-core/CAEJZ43iCipyOZddZAT1WCrtQ7g7VRLeokgLRGcYroK-TvGoGWw%40mail.gmail.com.


Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-27 Thread Marc Köhlbrugge
 https://github.com/rails/rails/issues/37555

I did find a few similar issues, but they all got marked as stale

https://github.com/rails/rails/issues/34408
https://github.com/rails/rails/issues/31996
https://github.com/rails/rails/issues/34093

I'm surprised this doesn't appear to be a bigger problem. Perhaps I'm not 
supposed to fragment cache my paginated results. Using collection caching 
(`render collection: @posts, cached: true`) might be performant enough.

On Sunday, October 27, 2019 at 2:55:18 PM UTC+1, richard schneeman wrote:
>
> Sounds like a bug. Can you move into an issue? 
>
>
>
> On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge  > wrote:
>
>> I'm running into a problem when using fragment caching with active record 
>> collections.
>>
>> Rails 6 uses a combination of collection.cache_key and 
>> collection.cache_version to determine whether a fragment cache is fresh or 
>> not. However, there is a scenario where the collection might change while 
>> collection.cache_key and collection.cache_version stay the same.
>>
>> It happens when you use a limit on the collection and then destroy one of 
>> those records, as long as it's not the most recent one. This way 
>> collection.cache_key will stay the same, because the query does not change. 
>> And collection.cache_version will stay the same as well, because the 
>> collection count will remain unchanged as does the maximum updated_at 
>> timestamp of the most recent record.
>>
>> I've build a sample app for demonstration purposes:
>>
>> https://github.com/marckohlbrugge/rails-6-collection-caching-bug
>>
>> The readme describes a way to reproduce the issue via the website itself, 
>> or the Rails console.
>>
>> Would this be considered a bug or expected behavior? Are there any known 
>> work arounds?
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonra...@googlegroups.com .
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com
>>  
>> 
>> .
>>
> -- 
> Richard Schneeman
> https://www.schneems.com
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/rubyonrails-core/6590103e-3528-4896-bc07-e6ff6a194bef%40googlegroups.com.


Re: [Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-27 Thread richard schneeman
Sounds like a bug. Can you move into an issue?



On Thu, Oct 24, 2019 at 7:52 PM Marc Köhlbrugge 
wrote:

> I'm running into a problem when using fragment caching with active record
> collections.
>
> Rails 6 uses a combination of collection.cache_key and
> collection.cache_version to determine whether a fragment cache is fresh or
> not. However, there is a scenario where the collection might change while
> collection.cache_key and collection.cache_version stay the same.
>
> It happens when you use a limit on the collection and then destroy one of
> those records, as long as it's not the most recent one. This way
> collection.cache_key will stay the same, because the query does not change.
> And collection.cache_version will stay the same as well, because the
> collection count will remain unchanged as does the maximum updated_at
> timestamp of the most recent record.
>
> I've build a sample app for demonstration purposes:
>
> https://github.com/marckohlbrugge/rails-6-collection-caching-bug
>
> The readme describes a way to reproduce the issue via the website itself,
> or the Rails console.
>
> Would this be considered a bug or expected behavior? Are there any known
> work arounds?
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com
> 
> .
>
-- 
Richard Schneeman
https://www.schneems.com

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/rubyonrails-core/CAFA5uRNRZQh2PiZ5uNoiBcbz5oj11FJhOurjJLmfwPkP%3D7Bo%3DA%40mail.gmail.com.


[Rails-core] [Rails 6] Bug in caching key generation when working with active record collections?

2019-10-24 Thread Marc Köhlbrugge
I'm running into a problem when using fragment caching with active record 
collections.

Rails 6 uses a combination of collection.cache_key and 
collection.cache_version to determine whether a fragment cache is fresh or 
not. However, there is a scenario where the collection might change while 
collection.cache_key and collection.cache_version stay the same.

It happens when you use a limit on the collection and then destroy one of 
those records, as long as it's not the most recent one. This way 
collection.cache_key will stay the same, because the query does not change. 
And collection.cache_version will stay the same as well, because the 
collection count will remain unchanged as does the maximum updated_at 
timestamp of the most recent record.

I've build a sample app for demonstration purposes:

https://github.com/marckohlbrugge/rails-6-collection-caching-bug

The readme describes a way to reproduce the issue via the website itself, 
or the Rails console.

Would this be considered a bug or expected behavior? Are there any known 
work arounds?

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/rubyonrails-core/4cc0ae69-c736-48d0-bd84-8b3f44dc879d%40googlegroups.com.