Skip to content

Fix memory leak in active record cursor#69

Merged
pedropb merged 1 commit into
masterfrom
fix-memory-leak-in-AR-cursor-next-batch
Mar 30, 2021
Merged

Fix memory leak in active record cursor#69
pedropb merged 1 commit into
masterfrom
fix-memory-leak-in-AR-cursor-next-batch

Conversation

@pedropb

@pedropb pedropb commented Mar 18, 2021

Copy link
Copy Markdown
Contributor

This commit patches the active record cursor implementation so that the
queries emitted by relation.to_a in next_batch are never cached.
This makes sense because it is a waste to cache results of queries like
SELECT ... WHERE id > ... LIMIT ... in an enumerator that is only
moving forward.

The uncached interface I make use of in this commit was introduced by
rails/rails#28867 under a similar context.

@pedropb pedropb changed the base branch from master to fix-rubocop March 18, 2021 17:01
@pedropb pedropb changed the base branch from fix-rubocop to master March 18, 2021 17:01
@pedropb pedropb marked this pull request as ready for review March 24, 2021 13:27
@pedropb pedropb force-pushed the fix-memory-leak-in-AR-cursor-next-batch branch from 218309e to 84218c4 Compare March 29, 2021 13:57
This commit patches the active record cursor implementation so that the
queries emitted by `relation.to_a` in `next_batch` are never cached.
This makes sense because it is a waste to cache results of queries like
`SELECT ... WHERE id > ... LIMIT ...` in an enumerator that is only
moving forward.

The `uncached` interface I make use of in this commit was introduced by
rails/rails#28867 under a similar context.
@GustavoCaso GustavoCaso force-pushed the fix-memory-leak-in-AR-cursor-next-batch branch from 84218c4 to 342df24 Compare March 29, 2021 14:22
@pedropb pedropb merged commit f752c66 into master Mar 30, 2021
@pedropb pedropb deleted the fix-memory-leak-in-AR-cursor-next-batch branch March 30, 2021 13:48
@shopify-shipit shopify-shipit Bot temporarily deployed to rubygems March 30, 2021 13:50 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants