Comments (5)
Fair enough :) I think the best way forward would be to look for a guaranteed solution rather than one that reduces the risk.
At least the bug is fixed now :D
from ranked-model.
Hi @fschwahn, that error rings a bell. I thought we'd fixed it or at least guarded against it. I assume you don't have a default value on that column as that would be prevent on boot.
Would you be interested in looking at a solution to this one? I suspect it'll involve locking of some kind :)
from ranked-model.
I've looked a bit into it, and I found the problem:
ranked-model/lib/ranked-model/ranker.rb
Lines 295 to 303 in c8ebb37
Calling reverse
on finder
loads the loads the ActiveRecord::Relation
into memory (ie. finder.loaded?
returns true). Every subsequent call to finder.first
does no DB lookup anymore, but takes the result from the loaded relation in memory. In concurrent settings this loaded relation might be outdated (in my example returns nil instead of a record).
Loading the entire relation into memory seems never a good idea here, as only one record is of interest, so an immediate fix would be to use last
so not the entire relation is loaded:
if (ordered_instance = finder.last)
I know too little about the code to tell if this has other side-effects, but it seems like a safe change.
That's a good idea in any case. However, in case current_first
already returned something, it is memoized in an ivar, and might also be outdated. So it might make sense to call reset_cache
at the top of rearrange_ranks
. However, it is much less obvious to me if this has other side-effects.
from ranked-model.
That makes a lot of sense :) Even with .last
I suspect there's still a chance for stale data unless we wrap all of this in a locking transaction?
The decision for .reverse
was a bit bizarre:
But it was a simplification of what came before it, though prior to that the code was genuinely trying to reverse the sort order of the query. That code it a lot simpler these days with modern Rails :)
I'll close this for now, but let me know if you have any more thoughts on how to tighten this up.
from ranked-model.
I suspect there's still a chance for stale data unless we wrap all of this in a locking transaction?
Yes, I think so. However I don't know much about pessimistic locking, and I'd be afraid of introducing deadlocks 😐
I'll close this for now, but let me know if you have any more thoughts on how to tighten this up.
One more idea I had was introducing an optional jitter (applied in rank_at_average
) to reduce the likelihood that 2 items are given the exact same row_order value (which is what ultimately leads to this issue here). However, that might not be desirable in situations where lots of re-ordering happens and the entire available space must be used.
from ranked-model.
Related Issues (20)
- Conficts with Mobility JSONB backend HOT 6
- undefined method `<' for nil:NilClass ranker.rb:215:in `rebalance_ranks' HOT 3
- Typo in Readme? HOT 1
- v0.4.6 Error: ActiveRecord::NotNullViolation: PG::NotNullViolation: ERROR: null value in column "row_order" violates not-null constraint HOT 5
- {column_name}_rank causes NoMethodError HOT 6
- Migration acts_as_list => ranked_model HOT 1
- Invalid SQL in `relative_rank` when order column value is nil HOT 2
- NonNilColumnDefault breaks running app when rank column has a default value HOT 7
- Publish new version to RubyGems HOT 5
- Order is incorrect when updating multiple records with accepts_nested_attributes_for HOT 5
- Creating records with ActiveRecord::Enum in rails 6.0 causes ActiveRecord::RecordNotUnique. HOT 5
- N+1 requests when outputting rank values HOT 6
- How to detect that `rearrange_ranks` or `rebalance_ranks` has been executed? HOT 3
- Is there a limit to the number of items to sort? HOT 4
- Failing to explicitly set position can lead to reordering on update HOT 3
- Unavoidable Rank N+1 Queries HOT 4
- how to find neighbor? HOT 9
- Another race conditions when inserting records HOT 14
- Super sorry. Wrong repo.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ranked-model.