playframework / playframework

The Community Maintained High Velocity Web Framework For Java and Scala.
http://www.playframework.com
Apache License 2.0
12.55k stars 4.1k forks source link

50 evolution file limit on SqlAnwhere database #6161

Closed skrigs closed 8 years ago

skrigs commented 8 years ago

Play Version (2.5.x / etc)

Play 2.4.6

API (Scala / Java / Neither / Both)

Both

Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)

Windows 10

Use uname -a if on Linux.

JDK (Oracle 1.8.0_72, OpenJDK 1.8.x, Azul Zing)

java version "1.8.0_51" Java(TM) SE Runtime Environment (build 1.8.0_51-b16) Java HotSpot(TM) 64-Bit Server VM (build 25.51-b03, mixed mode)

Library Dependencies

SqlAnywhere version 16.0.0.1691 database Jdbc driver is supplied with SqlAnywhere installation

Expected Behavior

Please describe the expected behavior of the issue, starting from the first action.

  1. Applying database evolutions

    Actual Behavior

Please provide a description of what actually happens, working from the same starting point.

Be descriptive: "it doesn't work" does not describe what the behavior actually is -- instead, say "the page renders a 500 error code with no body content." Copy and paste logs, and include any URLs. Turn on internal Play logging with <logger name="play" value="TRACE"/> if there is no log output.

  1. With an empty database
  2. Play application is started and successfully identifies evolutions need to be applied
  3. I proceed to apply the evolutions
  4. On the 50th evolution file an exception is thrown by the JDBC driver
  5. The Sybase JDBC driver reports "Resource governor for 'prepared statements' exceeded"

This is the database limit that is being hit: http://dcx.sybase.com/1101/en/dbadmin_en11/max-statement-count-option.html

I was successfully able to resolve this issue by explicitly closing the prepared statement in play.api.db.evolutions.EvolutionsApi::evolve::logBefore method by adding a ps.close() call after the ps.execute() method. Currently the prepared statement isn't being cleaned up until after the connection is closed, but this is too late for SqlAnywhere and the garbage collector hasn't had a chance to cleanup due to the small time window.

Reproducible Test Case

  1. Happens anytime 50 evolutions files are applied at startup on an SQLAnywhere database using default settings.
gmethvin commented 8 years ago

@skrigs Are you saying you've identified a fix already? Are you interested in submitting a pull request?

skrigs commented 8 years ago

We are currently on Play 2.4. Do you require the pull request for 2.5? I'm not sure what tests you would require since it is caused by the use of a proprietary database and I have never seen the issue on any other database I have used with Play.

gmethvin commented 8 years ago

@skrigs Typically if the logic is still similar in master you would submit a pull request to master and we'll backport to 2.5.x and 2.4.x. In this case it looks like the code hasn't changed much.

There's actually a bunch of code there that should probably be rewritten to:

val ps = ...
try {
  // do stuff with ps
} finally {
  ps.close()
}

I'm not sure if it's easy to test for, but that's just good practice to close prepared statements, so if the existing test suite passes I think it's fine.

gmethvin commented 8 years ago

That said we're more than happy to make this change ourselves; just wanted to give you a chance to make a contribution.

skrigs commented 8 years ago

On this occasion I think it might be more efficient if you guys make the change. Hopefully something more substantial presents it self where I can contribute code in the future.