<kares>
was wondering if you guys aren't obsessed with Rails tests
<kares>
I mean we sure managed to get the whole 1.3 line stable supporting several 3.x/4.x AR versions and at 8+ adapters without running AR's suite
<kares>
not saying its good - definitely should have run it occasionally :)
<kares>
not sure its worth the effort to get it to pass all - real-worlds users won't care about passing the AR suite, right?
<rdubya>
i guess from my perspective, the rails test suite is the functionality that users expect to see, so if we aren't passing those tests it means we have an inconsistency from the Rails adapters
<rdubya>
they also have a lot more coverage than arjdbc currently has and it seems like if we are trying to keep the gem small we'd want to have the rails tests cover most of it and only have unit tests for pieces that are additional to what active record provides, but maybe that thinking is flawed
vtunka has quit [Quit: Leaving]
<rdubya>
kares: it does seem like there are some tests that we just won't be able to have pass because they are testing for things that just work differently, so we probably won't be able to get it to 100% unless we make some PRs against active record to make the tests work across both
vtunka has joined #jruby
<kares>
rdubya: hey! you're right but than again there are production apps with 1.3 that surely do not pass the suite :)
<kares>
so what portion of #820 is the AR suite?
<kares>
(if its worth it - lot of test failures without it let's have but otherwise would hold off)
<rdubya>
The tests that it fixes are 2 that are explicitly testing for that statement cache stuff, so I'm fine leaving that off for now and just knowing that it is why those 2 break
<rdubya>
but i think we should have the cache in there
<rdubya>
I know that benchmark that I made isn't exactly real-world but it does show a significant improvement for basic lookups which are probably a good portion of most apps
<kares>
what about having JNDI e.g. with Tomcat configured to do stmnt caching, are you sure it would make sense?
<rdubya>
i'm not really familiar with jndi
<kares>
in theory there maybe most of the perf.improvements are just from avoiding Ruby -> Java SQL string conversion
<kares>
which might need a rework at the native level
<rdubya>
that's possible, but it is also eliminating the parse step on the db side
<rdubya>
which I'm sure accounts for part of it
<rdubya>
and is reusing the prepared statement object instead of creating a new one every time, though that may be offset by the need to call back to ruby and unwrap it
<kares>
so basically you just aren't closing it as you would have otherwise, right?
<rdubya>
yeah
<rdubya>
I believe the sqlite adapter is also using the statement cache now, but its using active record's logic instead of running it through jdbc
<rdubya>
enebo: can you verify that?
<kares>
and that might be what a JNDI configured data-source might mind (but I am also not 100% here - would need to do a real-world test)
<kares>
a PreparedStatement object still needs closing according to JDBC
<kares>
its an interesting idea thought - the PG driver might just be fine with it
<rdubya>
it gets closed now when the statement cache is emptied, but this is how jdbc expects you to use prepared statement objects, from what I can tell anyways
<rdubya>
I am new to the jdbc world so I'm learning as I go here :)
<rdubya>
we're actually fairly close to passing the whole AR test suite, I currently have 7 failures still showing and 3 of them are also failures with the arjdbc test suite, 2 of them are threading related that will hopefully be fixed with jruby 9.1.14.0 and 2 of them are ones that we likely can't get to pass without AR changes so I'm not worrying about them for now
<rdubya>
there's only 1 left that i need to try to track down and it is broken for postgres and sqlite, so not sure what is happening with that one
<kares>
cool - sounds great
<kares>
so if possible please hold off on this one
<kares>
still not finding time to get to spent a few days in a row on AR-JDBC but some folks might help with that ...
<rdubya>
I can wait on it until we do more testing or whatever you think we should do, but it does look like a potential 50% performance boost in some cases so it would be cool if we could include it at some point
<kares>
regarding PS not sure that it entirely true - JDBC pools might detect a prepared-statement that is open for longer periods as "leaking"
vtunka has quit [Quit: Leaving]
<kares>
actually do have perf.boost on master but the merge will be tricky and not all of it is unfortunately there :(
<kares>
no promises but we'll see
<rdubya>
from what I was reading in that thread that I linked in the PR, that is the only way to get a server to actually cache the query plan for postgres, but again, I'm new to this stuff so I could be missing something
<rdubya>
we actually explicitly disable prepared statements in our prod app because of our infrastructure so this stuff won't even help us lol
<kares>
yeah - also not sure. might take a look at the official driver - they did quite some changes in the driver lately
<kares>
should be releasing some of it (new jdbc-postgres gem versions) shortly
<rdubya>
cool
<rdubya>
I'm going to go back to trying to fix the rest of the arjdbc suite then, I was waiting until after this caching stuff was done because I expected it to be a lot more changes than it ended up being and for some of the failures, I wasn't sure what the expected functionality actually was, so I wanted to get the rails suite passing so I could tell what users would expect
<enebo>
kares: so I am just trying to understand your perspective here. You would rather not integrate into AR caching because we can use JNDI to get the same level of caching?
<enebo>
kares: and if JNDI is the path forward we do not need to customize a web container to get that (or use a special jdbc connection string?)
lance|afk is now known as lanceball
vtunka has joined #jruby
hamburger has joined #jruby
<hamburger>
Hey.
vtunka has quit [Quit: Leaving]
<kares>
enebo: the point it, that it seems quite wrong to me in terms of JDBC - think it could lead to issues with non-standard setups
<enebo>
kares: yeah I agree we need this to work with all JDBC-specified configurations
<kares>
not sure I have seen JDBC used in such ways in Java land
<enebo>
kares: so one question I have is why are we seeing perf go up by caching ourselves?
<kares>
assuming PG driver does not support server PS (which sounds weird - did not check), they pbly have a reason
<enebo>
kares: maybe it is out of date?
<kares>
maybe
<enebo>
kares: or was added later
<kares>
will look into that part
<kares>
or actually rdubya could just point a more recent driver onto hiw FS and try again ...
<enebo>
kares: I feel like we both have slightly different objectives on rails-5. I want all the Java things to work for advanced users but I want to push as much Ruby back into AR as we can
<kares>
enebo: yes :) I see that as a lost case
<kares>
regarding perf. cause you have the statement object and do not need to transcode a largish string Ruby -> Java
<enebo>
kares: well I am pretty sure I can get 90+% back to AR if I just pair with Aaron or another Rails person who will help make it happen
<enebo>
kares: ah yeah that does make sense
<hamburger>
I have a question about how JRuby works in a development environment when the program is run. Is there a .class file generated somewhere which the running program uses, or does it work differently?
<enebo>
kares: transcoding has been a bane for us
<kares>
aaah well that sounds good - my attempt on dealing with Rails was very frustrating ;(
<enebo>
kares: then that means we would see perf go way down if we process many results
<enebo>
kares: fwiw sqlite3 might be the only one where it works well
<kares>
not sure - just a guess
<kares>
am also out of sync but I was able to get perf. down to ~ MRI on master for MySQL/PG
<enebo>
kares: but even having a file like oid.rb seems like it should be able to disappear with a little work
<kares>
this difference just means there's smt really wrong
<enebo>
kares: Have you measured us vs MRI on rails-5?
<kares>
nope it was like 4.1
<kares>
than AR introduced more .rb crap/features so it went down
<kares>
and master does not have 5 support
<enebo>
kares: so the balancing point is having a working arjdbc vs an optimal one atm
<hamburger>
Anyone know anything about what I asked about?
<kares>
enebo: that code for me is just asking for trouble ... and Rob said its due 2 tests
<enebo>
hamburger: by default we start by interpreting Ruby code but then we dynamically generate bytecode in memory
<enebo>
hamburger: so in a sense we make the in-memory equivalent of .class for a Ruby method (just an example)
<kares>
I am not really sure it makes sense ... also the native side just looks weird -> too much .rb details leaking there
<hamburger>
Does it happen before anything else?
<enebo>
hamburger: we also have AOT but this largely is just for when you need a .class explicitly...there is not benefit to it
<kares>
but hey if you feel like it makes sense, well go for it - hoping to look at it but I am making zero promises :)
<enebo>
kares: so there are two things there
<enebo>
hamburger: sorry I don't understand what you just asked
<hamburger>
I'm writing a language adapter for Forge to allow JRuby to be used for mods, but it's not really picking up the Ruby code. However, if I compile it to a .class, it works fine.
<enebo>
kares: 1) that config option is just to pass a test...that maybe can go into the rake task via config/
<enebo>
2) generic stmt cache in Rails does seem to remove that transcoding cost and in pgsql seems to actually make server-side pstmts work
<enebo>
so I think those are different
<kares>
enebo: yeah I forgot about that - there's a place were it can/should go yes
<kares>
was only mentioning 2)
<enebo>
hamburger: ah I see have you seen the project Purugin?
<enebo>
hamburger: it is not forge friendlty though
<hamburger>
I have not, but I just Googled it.
<enebo>
hamburger: but you can use jrubyc if you need it to be a proper class
<hamburger>
I have tried using that, and that's the only way I've gotten it to work so far.
<enebo>
hamburger: there is a become_java! method which you can annotate some methods with java signatures but I believe that will not work with no-arg constructor patterns that some Java APIs use
<kares>
also if you guys look into JDBC pools -> they seems to provide the PS instance caching on their own
<hamburger>
My files do all have "require 'jruby/core_ext'" at the top of them, so they should all have "become_java".
<enebo>
kares: will it even work in Torquebox
<kares>
enebo: not sure what you're asking?
<kares>
in any Java server it can be get into working
<enebo>
kares: statement cache from tomcat not workin on non-tomcat things
<kares>
obviously not out-of-the box in puma
<kares>
enebo: that's an example, there's other pools
<enebo>
kares: ok so I think we want both to be supported since it seems many people do just fire up puma
<kares>
but if you're asking that than yes Tomcat''s JDBC pool works outside TC
<enebo>
kares: oh yeah I don't doubt it but unless we ship it then we cannot really use it. Although other users can opt to use it and we can make it easier to use in that case
hamburger has quit [Quit: Page closed]
<kares>
yes - if PS is enabled I do not want to break .war deployments with JNDI or have surprises there
<enebo>
kares: yeah definitely. I agree
<kares>
well for me, it seemed like premature work
<kares>
that is why I asked Rob to hold off
<enebo>
kares: well at this point it can be held
<rdubya>
kares: I think we could disable it if we detect a jndi connection (if we can do that reliably)
<kares>
but I do not want to discourage you guys since you're at least pushing it somewhere
<enebo>
kares: yeah is detection even possible?
<kares>
so if you feel like I am opinionated please feel free to merge and let's deal with it later than :)
<rdubya>
kares: I'm fine with sitting on it for now and we can focus on the other pieces that still need to be finished
<enebo>
I would like that PR to change even if you were for it
<enebo>
I prefer the idea of passing in the cachedStatement vs a boolean since the top of exec_query in Java is just essently callMethods
<enebo>
we might as well do that out in Ruby
<kares>
yy - that looked weird to me too
<enebo>
but if we had detection we could then have two versions in Ruby where one would manage cached stmt and other would defer to JDBC
<kares>
well at least an option to disable that 'internal' statement cache would be najs
<kares>
would certainly make me happier
<kares>
have seen it before but somehow always decided against porting such pieces over from AR
<enebo>
kares: I would be happy if we just had a way of knowing whether JDBC was handling it
<kares>
(although my assumptions that they make very little sense the way they are might have been a bit too far)
<enebo>
kares: if we knew that then we could then enable internal but only in that case
<kares>
nah that's tricky
<enebo>
yeah I figured since there are n impl os pools
<rdubya>
we could also hold off on it for 5.0 and look to add it in for 5.1 support
<enebo>
I was still hoping the standard had some boolean somewhere :)
<rdubya>
since it seems like something we want to put more thought into before landing it
<kares>
1 check jndi:/data_source: config 2 try unwrapping the real conn and compare with the wrapper
<enebo>
hahah
<enebo>
kares: yeah and that is imperfect as well
<kares>
none of these work 100% reliable - there are cases were it might break for both
<enebo>
kares: so one use case I want to address is Puma specifically
<enebo>
people evaluate us just using that
<enebo>
not all the time obviously but it is a common server
<enebo>
so perhaps we make a property to enable internal caching and have it off by default
<kares>
yeah I can see from your puma perspective that code makes perfect sense :)
<enebo>
unless we see we are running on webrick or puma
<enebo>
and we can have a perf tuning wiki page perhaps explaining that people can pick an option
<rdubya>
didn't rails just switch to puma as the default webserver?
<enebo>
bbrowning: how does torquebox do stmt caching?
<rdubya>
in our company we develop on puma but run torquebox in production
<enebo>
ok so I think a safe outcome at this point is that for Puma/webrick we should have a code path to enable internal stmt caching
<enebo>
A less safe outcome is how to organize code so that we can have these two different paths
<enebo>
second sentence should not use the work outcome but you both understand :)
<bbrowning>
enebo: torquebox doesn't do anything special with statement caching. You can define and use a JDBC pool in the underlying jboss bits.
<kares>
OK - as I said I'm fine with having it on by default
<enebo>
bbrowning: ah but does that mean you edit xml?
<kares>
marking that I shall update/release newer postgre drivers, we shall maybe retest there or check out its configuration
<bbrowning>
enebo: it depends on the exact version but yeah, ultimately you're probably editing some java xml configs
<kares>
enebo: yes it does :)
<enebo>
bbrowning: how about the nearly released version of tb? :)
<kares>
does that mean no Ruby shop would configure? :) nope
<enebo>
kares: yeah that is sort of why I am asking
<bbrowning>
enebo: I'm not sure you can even configure jdbc datasources with torquebox 4. I've never tried it
<bbrowning>
you for sure can with 3 because it can run the full java app server underneath
<enebo>
ok well this makes me feel a little better with having a config option but having it default on then
<enebo>
so if you are an advanced user we can make sure they can disable this but the default out of box will use internal
<kares>
so you guys are already PS on by default on rails-5 ?
<enebo>
yeah
<enebo>
kares: I think pretty much everything is a PS
<kares>
I mean in general - AR switched somewhere around 4.x but AR-JDBC didn't follow ;(
<kares>
ok, thanks
<enebo>
rdubya: right?
<enebo>
kares: sometimes I forget more than I ever knew but it seems all code paths default to prep'd on sqlite
<enebo>
kares: the exec_query code now will prep even binds of []
<rdubya>
enebo: yeah, i think 4.2 arjdbc forced it to fals
<rdubya>
but rails defaulted it to true
<kares>
yeah I have fish memory so NP - basically you can not count on what I said 2secs ago :)
<kares>
rdubya: yep that forcing sounds like my work on not having time to get all the adapters green
<rdubya>
kares: makes sense, it took a bunch of work to get it working for rails 5 and it still isn't 100% there :(
<kares>
its amazing how far you took it - recall last time I tried it was failing all over the place, so I rather never tried since
<kares>
users didn't complain much - so that is why I am making the point of not going crazy about a test or two :)
<enebo>
sqlite3 is at 6/2 on AR tests and I am at 3/1 with MRI??? so sqlite3 is nearly finished too
<kares>
that is 333% done in my math!
RC has joined #jruby
<rdubya>
lol
<enebo>
heh
RC is now known as Guest90083
<kares>
any news on MySQL?
<kares>
would love to look at that (and actually some folks want me to) but work load is massive these days
Guest90083 has quit [Client Quit]
<enebo>
kares: JRuby work?
<enebo>
kares: I am almost ready to start and I might since I am frustrated and could use distraction
<kares>
frustrated with what :) ?
<kares>
yep - Rails ~ on JRuby ... but I sure am considering not taking any Rails work anymore :)
<kares>
so I might end up job less again in a few months ... which means doing OSS if I feel like it in the morning :)
doodle has joined #jruby
doodle has quit [Client Quit]
hamburger has joined #jruby
hamburger has quit [Ping timeout: 260 seconds]
shellac has quit [Quit: Leaving]
lamorim has joined #jruby
swills has quit [Remote host closed the connection]