kares
commented
6 years ago simply silence the JUL logger. that should work, however this is a driver thing - not much AR-JDBC's fault
Open meetme2meat opened 6 years ago
kares
commented
6 years ago simply silence the JUL logger. that should work, however this is a driver thing - not much AR-JDBC's fault
enebo
commented
6 years ago @kares I am not sure most users would agree with you though. If I use something which is meant to be giving a particular feel (e.g. AR clone), then I do not expect to see the implementation details leak out onto my screen. Another side of this is if these details are important to how the db is actually behaving perhaps we should be connecting this to rails logging? Final thought is many people who use JRuby are not Java programmers and do not have any idea how Java logging works to disable it.
kares
commented
6 years ago @enebo hey, did check that there's the same issue opened at pgjdbc. the error message got introduced in some recent driver version and they seem to be doing something about it (according to comments on the tracker). it wasn't printing this prior to version 42. believe @meetme2meat noticed that as well and will agree there's no need to double the investigation here.
while your ideas seems legit and fine, I do not see pushing them forward - we did not even restore JNDI support (yet) and with caching at .rb side I'm not sure anyone has thought it through ...
Another side of this is if these details are important to how the db is actually behaving perhaps we should be connecting this to rails logging?
if you ask me - we definitely do not want to do this esp. if you're for that particular AR clone feel :)
Final thought is many people who use JRuby are not Java programmers and do not have any idea how Java logging works to disable it.
this isn't the case here as user understood he's using a Java driver and opened an issue at the proper place.
will wrap up slowly ... and I am leaving this up to you than.
enebo
commented
6 years ago @kares I am still a little confused about this. I only saw the linked issue over there yesterday (which really looks like wontfix from the comments) but this morning I noticed the PR which will reduce the log to warn (which I am guessing warns do not log by default). So, if they are changing it back then that means a future version will again not display to console. Don't we bundle the version which does this though? We will need to at a minimum change the version of the pg driver OR silence the noise OR ignore it until a new version comes out then update the version. Seems like all paths require some action on our part.
I was not really sure how important it is to propagate the log errors so that was a question. Pushing them through to AR probably would also generate user reports; so I can see that potentially opening up another source of user reports.
While the reporter may know how to work around this issue I am hoping he is not our sole postgresql user. I mostly brought up that last point because we will just end up getting this reported again until we change something.
kares
commented
6 years ago believe its not important much - we do not care about native SQLException by (default) now. in this case I thought it to be obvious - AR call generated a non-existent DB error and we do not care.
we have a work-around locally (for tests) but that is really unfortunate to be the resolution by AR-JDBC as users who might have tuned their JUL with a configuration (e.g. for driver debugging) we would blow over.
... the only 'good' option would be to fork the driver but there's activity on driver's end to resolve. that is why I closed the issue here.
on a related end the (recent) increased usage of PG's JDBC API has gotten us tighter to a driver version, which wasn't the case in the past but we're slowly heading there - so maybe forking is an eventual option :)
enebo
commented
6 years ago @kares I wonder if we could just go back one version until the next one is out. I don't think we updated this driver before 5.x work for several years. Perhaps being a little out of date and not having spurious errors going to console is preferable? (I have not see what was fixed so I guess that is a big factor in that idea).
I will talk to you on irc about what you mean about API. I think you mean we are using newer features of pg itself which only newer APIs support but I am unsure.
meetme2meat
commented
6 years ago @kares @enebo Thanks for help.
@kares like you suggested. All that is the need to do is, this..
java_import "java.util.logging.LogManager"
LogManager.get_log_manager().reset() I don't see those logs after I do this... But I'm certainly not sure if this is the correct way to do it.
Every since we updated activerecord-jdbc-adapter to 50.0 we are seeing JDBC error on our STDOUT.
While I understand the error part but its kind of very frightening and confusing at times (when the code has all the rescue enable to log the error to a file) to see this on our STDOUT.
Plus I'm not sure that extra IO operation takes some performance hit when a Large number of db failure issue happens and when we need to respond as very fast.
I happen to find the source of the JDBC adapter where this happens here and the Logger is defined over here
But... I'm not sure how to handle this ..
In the end, I'm really not sure if this is the right place to report this error.