Replies: 5 comments 13 replies
-
|
Maybe a slight tangent, but if I implemented schema caching, would that prevent this specific error because the schema is guaranteed to be available? (Obviously we’d still see some error when the DB connection glitches, but this particular issue was harder to track down than a ‘regular’ DB connection problem 😅 ) |
Beta Was this translation helpful? Give feedback.
-
By default, a failure to load the model schema should raise an exception, it shouldn't be silently swallowed. If it is being silently swallowed, my guess is you have set require_valid_table to false, which is a terrible idea. Alternatively, you are using a very old version of Sequel and should upgrade.
This would be expected if Sequel cannot determine the table schema. Obviously, this is a bad thing, which is why Sequel sets require_valid_table to true by default.
Yes, the schema_caching extension would probably help prevent this particular type of error. One potential issue I see is that the UnableToSend exception is not treated as a disconnect error. What happens on future use of the same connection? Do you get a different error that is treated as a disconnect error? I would assume you must, otherwise you would have a disconnected connection in the pool that would continuously cause problems every time it is used. You could try the following patch and see if helps, though it is unlikely to be related to this specific issue: index fe9292c30..0eeec582a 100644
--- a/lib/sequel/adapters/postgres.rb
+++ b/lib/sequel/adapters/postgres.rb
@@ -52,6 +52,9 @@ module Sequel
if defined?(::PG::ConnectionBad)
DISCONNECT_ERROR_CLASSES << ::PG::ConnectionBad
end
+ if defined?(::PG::UnableToSend)
+ DISCONNECT_ERROR_CLASSES << ::PG::UnableToSend
+ end
DISCONNECT_ERROR_CLASSES.freeze
disconnect_errors = [ |
Beta Was this translation helpful? Give feedback.
-
We’ve been able to recreate it in a minimal local console, connected to the production DB. It seems to be exactly what you say: the issue occurs when the connection has been idle for a bit. Possibly the ‘consistent’ failures in production are just because the Resque jobs are sporadic. That might also explain why the webapps are not affected, because they are getting hit by healthchecks which then touch the DB so maybe that’s keeping them alive. I’ll look at those extensions and see if they help, thank you. (Yes, we would certainly like to move the DB to the same location as everything else for a bunch of reasons, but that’s not straightforward right now.) This is What’s more, in the console where we recreated the error it does raise, and does recover the connection so subsequent queries are fine. I’ll keep digging in Resque to try and figure out what’s different there. I also think possibly we’re initialising two connections to the database when Resque boots, could that leave Sequel unable to recover? In any case I’m going to try to remove that behaviour. Finally, I noticed that these errors are almost always happening immediately after we run |
Beta Was this translation helpful? Give feedback.
-
You could try using the connection_validator extension, which validates connections before use. I'm not sure how this is an issue with idle connections, unless you are not loading the related model class at application startup.
This lends more evidence to the related class not being loaded at startup. If you are delaying loading the class until a specific job runs, then what is probably happening is the class partially loads, but raises an exception. Then later, some other code uses the partially loaded class. The simplest workaround would be to load all related classes at application startup. However, if that will not work, you could try working around this issue by switching from: class ModelName < Sequel::Model
# ...
endto: ModelName = Class.new(Sequel::Model(:table_name)) do
# ...
endThe difference between these two cases is if there is an exception raised while defining the model, in the first case, you are left with a partially loaded class, while in the second case, you are not, as the constant is not set until after the class is fully defined.
It looks like the disconnect handling may be working at intended then. Can you tell from the logs if the PG::UnableToSend exception raised as a Sequel::DatabaseDisconnectError, or a regular Sequel::DatabaseError?
I would guess so as well. |
Beta Was this translation helpful? Give feedback.
-
Ahah! I think this might be it – these models are being loaded by Zeitwerk on demand, which must be post-fork in Resque. That sounds like it fits the pattern here: this problem keeps recurring when this particular model is loaded. Possibly the way Zeitwerk is configured is also swallowing the error in some way – I’ll look into that. Okay, I think there are a few possible solutions for us to test here (and maybe use more than one of them if we find multiple options that work):
I can’t tell, it’s only logging I’ll also try adding |
Beta Was this translation helpful? Give feedback.
-
Eager loading all models before processing requests seems like the simplest fix. Might even save on memory if done pre-fork. Caching the table schema should speed up loading model classes. I do this in all of my production applications. You probably want to use the connection_validator extension, since it sounds like your database connections are not reliable. Trying to raise/catch does not seem like a good approach for this case.
If it's possible to print the exception class, exception message, and full backtrace for these exceptions, that would be useful. Hopefully that would allow me to see if changes should be made to Sequel to handle this case.
This setup looks OK. |
Beta Was this translation helpful? Give feedback.
-
I’ll look at getting a full backtrace from Resque, but when re-creating locally the logging is slightly different: This looks like it’s working as intended, I think, because I also realise now our console is using 5.46 whereas Resque is on 5.34, so we definitely need to upgrade/homogenise this all. Maybe this is something you already fixed, in which case apologies for not trying to upgrade before getting in touch! |
Beta Was this translation helpful? Give feedback.
-
|
Hi @jeremyevans, thanks so much for your help, the changes we discussed above seem to have solved this issue. I was able to get a full backtrace for the original exception by redefining The lines in our application code here don’t look significant. They are: 48: Entrypoint for the Resque job Backtrace line 34 is the definition of the class which blows up later on: The Zeitwerk lines in between our application code lines are in Zeitwerk’s override of |
Beta Was this translation helpful? Give feedback.
-
|
To see if this problem is somewhere in Sequel, can you override |
Beta Was this translation helpful? Give feedback.
-
|
Overriding I see |
Beta Was this translation helpful? Give feedback.
-
|
In this case raising would have been good, yes, but I take your point about backwards compatibility, and I’ve no experience with models where the schema cannot be determined. Perhaps a compromise would be to log a more explicit warning? Then it wouldn’t actually break any code, and there could be an option to suppress the warning if the model is known to be good without a schema. That might encourage people (a little) to either suppress the warning or turn on strict mode depending on the context. It does seem strange that the second query is failing and not the first. The connection seems to always initially fail after ~10m idle. Is it possible that Sequel would load the columns during app startup, and then they would have already been cached by the time the schema is queried? Otherwise I can’t see how this would happen either. A couple of other Sequel::Models do get loaded during startup, but they have no relations to the Model which fails to load. |
Beta Was this translation helpful? Give feedback.
-
|
Logging a warning does seem like a reasonable compromise. Maybe I can use that approach, and switch to an error in Sequel 6 if nobody complains. Note that it may not be too surprising that the second query is failing and the first isn't, because the same connection isn't necessarily used for both queries. It's also possible the there is no query to get the columns, because the result is already cached. |
Beta Was this translation helpful? Give feedback.
-
|
I looked into this more. It turns out the schema query is actually issued first, which is why it's more likely to hit the disconnect error. The query to get the columns may not even be issued if the query to get the schema returns as expected. If the query to get the schema fails due to a disconnect error, Sequel will issue a query to get the columns, but it won't try parsing the schema again, causing this error. I think that behavior should be changed, not simply warned. I'll work on this today and see if I can fix this issue with minimal changes to backwards compatibility. |
Beta Was this translation helpful? Give feedback.
-
|
I pushed a couple changes. One will treat disconnect errors like connection errors. Another is the addition of a |
Beta Was this translation helpful? Give feedback.
-
|
Awesome! We’ll incorporate |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Hello, we’re having occasional database connection issues that are nothing to do with Sequel, but they are causing a knock-on effect in Sequel which I don’t know how to recover from.
What happens is:
PG::UnableToSend: SSL SYSCALL error: EOF detected: SELECT \"pg_attribute\".\"attname\" AS \"name\", CAST(\" ... etcPG::UndefinedColumn: ERROR: column \"customer_id\" does not existWhat I’d like to know is how I ought to be recovering from this? Nothing is raised when 1 fails, only when 2 does, so I don’t see an opportunity to retry at that point. If I rescue
PG::UndefinedColumnthen willDatabase#schema(:billing_infos, reload: true)reload the schema that Sequel::Model also refers to?If I always want to retry something like this at least once, is there a way to tell that to Sequel? I’m happy to structure higher-level code so it always tries to rescue and repair the situation, but maybe I’m missing something here.
Notes
The error in 2 is strictly correct, column
customer_iddoes not exist. But that’s not what’s being asked for, as seen in the Ruby code when this error occurs:Here
invoiceis a Sequel::Model instance,payment_methodis an object we got from Stripe’s API, and the relationsinvoice.useranduser.billing_infosare configured and work. Cruciallypayment_detailsis a JSON column, socustomer_idis supposed to be a key in a JSON object, not a column name.This code works as intended most of the time!
Beta Was this translation helpful? Give feedback.
All reactions