find_by_sql with UPDATE performed on read-only slave

[jacob-s-son]

Hi, guys!

Ran into problem with delayed_job_active_record, particulartly this query is executed on read-only slave:

self.find_by_sql(["UPDATE #{quotedTableName} SET locked_at = ?, locked_by = ? WHERE id IN (#{nextScope.to_sql}) RETURNING *",now,worker.name])

Since had no response at delayed_job repo, maybe You are willing to comment could it be an Octopus issue (maybe ActiveRecord's ?).

Issue in delayed_job repo:
collectiveidea/delayed_job_active_record#34

[sobrinho]

I'm not convinced that's a problem on octopus since find_by_sql means a read and not an update.

I'm checking if there is an alternative on active record to that case ;)

[lawrencepit]

Agreed that an UPDATE should not be send through a find_by_sql, however it also doesn't work using self.connection.update("UPDATE ...").

It would be nice if Octopus would switch connection to the master database for update, update_all, reload, delete, delete_all, destroy and destroy_all. Basically same implementation as the transaction method in proxy.rb.

In addition, given everything is pumped through execute and exec_query, those two could be hooked into and switch to master if sql =~ /^\s*(insert|update|delete|create|alter|rename|drop|add|change|begin|commit|rollback|savepoint|release)/i. Obviously that's getting into a bit of a shady area, but it would catch this issue with delayed job. I've seen a lot of gems where they do obj.connection.execute("UPDATE ...") statements, which is fine in a way, but atm Octopus is not dealing with that.

I wouldn't mind whipping up a patch for this.

[sobrinho]

@lawrencepit I would be happy to merge

[sobrinho]

Closing due to lack of movement but I would be happy to merge, just mention this issue on the PR.