Skip to content
This repository

Separate slug sequencing from slug generation #200

Closed
norman opened this Issue December 19, 2011 · 7 comments

3 participants

Norman Clarke Philip Arndt Lee Hambley
Norman Clarke
Owner

The current slugging code confuses and mixes the concepts of sequencing and generation. Ideally it should be possible for a developer to provide their own uniqueness resolution strategy other than simple adding a sequence.

Also, it's sometimes necessary to lock the model's table in order to properly sequence. Some non-locking solution should be provided - in previous FriendlyId this was done by making the sequence an external column, but this changed in 4.0 to provide more user-friendliness.

This is probably the worst area of FriendlyId's codebase and needs a good look-over.

Lee Hambley

This is probably the worst area of FriendlyId's codebase and needs a good look-over.

I have an application which suffers significantly from this, perhaps we could hash out some ideas together?

Norman Clarke
Owner

Yes - that would be great.

This was much less of an issue with FriendlyId 3, because the sequence was stored in a separate column and could be incremented inside a transaction. In the interest of simplicity I had hoped to avoid requiring users to make two columns, but without that I'm uncertain what other approach would work well. Perhaps FriendlyId could add a macro to ActiveRecord::Migration like Devise does, so that users would only have to add one line of code to their migrations, but FriendlyId would handle making the column or columns it needs.

Lee Hambley

I'd like that approach, as it is the full table locking (in any of PostgreSQL's exotic locking modes) doesn't work for me. (Every one takes the site offline, but I am running a routine that performs a full table lock about once a second.)

I've noticed that the Rails app hangs silently, I suspect this is an issue with the PostgreSQL driver not handling any kind of timeouts/logs from trying to read locked tables, that's annoying, to say the least! This is something I'll take up with the PostgreSQL author, separately.

I'd certainly vote for something outside of the normal flow, the first thing that comes to mind would be an alt. implementation people would be able to use using a simple redis key for storing the next ID. In the case that the cached key is blank, it could be looked up from SQL once, stored into Redis and then using their INCR command, atomically incremented.

That's the direction I'd like to go, and for those without redis in their stack, there should be some alternative (I can immediately think of something SQL alt. column based, as well as something using a native PostgreSQL SEQUENCE.

For people like myself, with both PostgreSQL, and Redis in the stack, I'd have two clean and sane options to use, and for those without, there would be a way to use a separate column, finally out-of-the-box there would be a simple, naïve implementation in Ruby, so the out of the box experience for the Rails 101 tutorial projects is still super easy.

What do you think? If you can complete the original scope of this ticket, I'll contribute Redis and PostgreSQL Sequence implementations?

Lee Hambley

Just some notes further to the above:

Documentation for Redis INCR
Documentation for PostgreSQL CREATE SEQUENCE

From the top of my head, here's how a naïve implementation might look:

module FreindlyId

  class UniqueSequence

    attr_reader :model_class

    def initialze(model_class)
      @model_class = model_class
    end

    def next
      make_ready
    end

    private

      def highest
        # See existing implementation
      end

      def wildcard
        # See existing implementation
      end

      def make_ready
        return if ready?        
      end

      def ready?
        true
      end

  end

  def RubyUniqueSequence < UniqueSequence
    def next
      super
      # See implementation in SlugGenerator
    end
    private
      def ready?; true; end
  end

  def RedisUniqueSequence < UniqueSequence

    def next
      super
      next_from_redis
    end

    private

      def next_from_redis
        redis.incr redis_key_name
      end

      def redis_key_name
        "friendly_id:unique_sequence:#{model_class.underscore}"
      end

      def make_ready
        redis.setnx redis_key_name, highest
      end

      def ready?
        redis.exists redis_key_name
      end

      def redis
        # Configurable
      end

  end

end
Norman Clarke
Owner

Solving the concurrency problem isn't the hard thing, it's solving the simplicity problem - I want it to be as easy as possible to set up FriendlyId, which I why I tried doing this with only one column. The solution you're proposing actually makes things a lot more complicated, and I don't think it's the right approach. Why add a dependency on Redis in the mix when you can do exactly what you want to do with MySQL, Postgres, etc? Including it as an option doesn't make it any simpler, because then I have 2 different implementations of the same thing to maintain, test and respond to bug reports about.

Lee Hambley

My point was that out of the box, for most people the Ruby version is good enough, unless one runs parallel processes.

If one runs parallel processes, that very often means Redis (and resque). A backend with an exceptional concurrency model, why not use it? (Edit: I mean, make it possible to use it.)

So I was proposing that if you pick an API that you are happy with, it would be trivial for others to contribute, or maintain separately unique key resolvers that work using the best technology available.

The PostgreSQL sequence idea came out of a conversation I had with a colleague who's our DBA who just said "stupid ruby programmers, just use a sequence", he might be right.

Finally to say that, I probably would prever a Redis solution (which I don't even mind writing) to a transactionally-locked-sequence-column solution, and the work to refactor friendly_id to have SlugGenerator and UniqueSequencer.new(MyModel).next shouldn't be too challenging.

Speaking of your wish for simplicity, why not simply ship the Ruby resolver (the current implementation, refactored) for the 90% of people for whom that is good enough, and offer an API for configuring which SlugGenerator is used, and with which UniqueSequenceResolver

Thoughts? (I've forked to look at refactoring a UniqueSequence implementation out of the existingSlugGenerator, and see if I can keep the beginner API the same, whilst offering the possibility to have them pluggable without burning the (very important, actually) simplicity of your library.)

Philip Arndt
Collaborator

Closed by #243

Philip Arndt parndt closed this February 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.