Hilfe! Datenbank Verbindungen hängen

[port29]

Hallo,

ich nutze DBCP, damit mein Servlet mit einer MySQL Datenbank Verbindungen aufbauen kann. Für die Datenbank Verbindungen habe ich deshalb folgende Klasse geschrieben:

public class DBConnection {
	private static BasicDataSource ds;
	
	public Connection getDataSource(){
		
		if(ds==null){
			ds = new BasicDataSource();			
			ds.setDriverClassName("com.mysql.jdbc.Driver");
			ds.setUsername(Config.get_mysql_username());
			ds.setPassword(Config.get_mysql_pwd());
			String url = "jdbc:mysql://" + Config.get_mysql_host() + ":3306/"+Config.get_mysql_dbname();
			ds.setUrl(url);
			ds.setMaxActive(10);
			ds.setMaxIdle(3);
		}
		try {
			Connection conn = ds.getConnection();
			return conn;
		} catch (Exception e) {
			e.printStackTrace();
			return null;
		}
		

	}
}

Sinn des Servlets ist es einfach nur RPC Calls auszuführen und Daten zurück zu liefern. Eine der Funktionen des Servlets sieht z.B. so aus:

	public UserData getAdvancedUserData(String userKey, UserData userData) {
		DBConnection db = new DBConnection();
		Connection con = db.getDataSource();
		
		try {
			String _userID = DB.checkUserKey(userKey, con);
			
			UserData result = DB.getAdvancedUserData(userData, _userID, con);
			con.close();
			
			return result;
		} catch (Exception e) {
			e.printStackTrace();
			return null;
		}			
	}

Es funktioniert eigentlich ganz gut, nur habe ich nach einiger Zeit irgendwann das Problem, dass auf einmal garnichts mehr geht. Und es scheint an

Connection conn = ds.getConnection();

zu liegen. Tomcat zeigt auch an, das zahlreiche Threads busy sind. Habe ich irgendwelche logischen Fehler in den Klassen drin oder woran liegt das?

 

[zeja]

Treten bei dir Exceptions auf? Du solltest auch im Falle einer Exception deine Connection wieder schließen, also packt das conn.close mal in nen finally Block.

 

[Oliver Gierke]

Kleiner Designhinweis:

Mach doch aus der DB Klasse, die lauter statische Methoden hält eine richtige. Das wär dann ne Implementierung des DAO Patterns:

public class UserDAO { // formerly known as DB

  private DataSource dataSource;

  public UserDAO(DataSource dataSource) {
    this.dataSource = dataSource;
  }

  public String checkUserKey(String userKey) {

    try {
       Connection connection = dataSource.getConnection()

       // deine Abfrage
    } catch (SQLException ) {
       // Exceptionhandling
    } finally {
       // sauberes(!) Resourcenschließen - Connection, Statements, ResultSets usw.
    }
  }
}

Im Servlet reicht dann:

public class Foo implements HttpServlet {

  private UserDao userDao;

  public void init() throws Exception {

    BasicDataSource dataSource = new BasicDataSource();
    // weiteres setup

    this.userDao = new UserDao(dataSource);
  }
}

Das ist zum einen wesentlich objektorientierter, du hast das Exceptionhandling nur in einer Klasse, das UserDao ist testbar (weil die DataSourceImplementierung austauschbar ist).

Noch schöner gehts wie hier beschrieben. Das ist ungefähr das gleiche, nur dass die das JDBC Template noch den ganzen try / catch Quatsch in den Methoden abnimmt.

Gruß
Ollie

 

[port29]

Hallo,

danke für die Antworten. Tatsächlich scheint es so zu sein, als ob wenige Exceptions fliegen und dabei die Verbindung nicht geschlossen wird. Aber wenn ich die Verbindung mit einem finnaly block schließe, dann habe ich ca. 100 Mal so etwas im Quellcode stehen:

catch (Exception e) {
			e.printStackTrace();
			try {
				con.close();
			} catch (SQLException e1) {
				e1.printStackTrace();
			}
			return null;
		}

Das ist natürlich nicht wirklich angenehm zu lesen. Ich bin eigentlich davon ausgegangen, dass eine Verbindung nach einer bestimmten Zeit automatisch wieder geschlossen wird. setRemoveAbandoned gibt es zwar, aber das scheint bereits veraltet zu sein.

Sobald ich etwas mehr Zeit haben werde, werde ich dem Designhinweis natürlich folgen und aus den statischen Methoden "normale" machen. Aber momentan ist da leider das Problem mit der Datenbank

 

[zeja]

Wenn dann so:

catch (Exception e) {
	e.printStackTrace();
}
finally{
	try {
		con.close();
	} catch (SQLException e1) {
		e1.printStackTrace();
	}
	return null;
}

Und fang besser jede Exception einzeln und nicht allgemein Exception. Das ist sehr unschön.

Aber ja: Der Code mit schließen der Verbindung im finally Block ist nicht schön zu lesen. Aber du kannst dir auch eine Util Klasse machen die eine Methode close(Connection) enthält und die Exception dann selber fängt... dann wirds ein wenig übersichtlicher.

Kannst dir auch mal http://commons.apache.org/dbutils/ anschauen.

 

[Oliver Gierke]

Hallo,

danke für die Antworten. Tatsächlich scheint es so zu sein, als ob wenige Exceptions fliegen und dabei die Verbindung nicht geschlossen wird. Aber wenn ich die Verbindung mit einem finnaly block schließe, dann habe ich ca. 100 Mal so etwas im Quellcode stehen:

catch (Exception e) {
			e.printStackTrace();
			try {
				con.close();
			} catch (SQLException e1) {
				e1.printStackTrace();
			}
			return null;
		}

Das ist natürlich nicht wirklich angenehm zu lesen. Ich bin eigentlich davon ausgegangen, dass eine Verbindung nach einer bestimmten Zeit automatisch wieder geschlossen wird. setRemoveAbandoned gibt es zwar, aber das scheint bereits veraltet zu sein.

Sobald ich etwas mehr Zeit haben werde, werde ich dem Designhinweis natürlich folgen und aus den statischen Methoden "normale" machen. Aber momentan ist da leider das Problem mit der Datenbank

Genau aus diesem Grund gibt es das JDBCTemplate . Kein Boilerplatecode, kein X-maliges Exceptionhandling.

Nochwas: dein ...try { conn.close()... gehört in den finally block. Sonst machst du die connection nicht zu, wenn das Ergebnis korrekt ist. Dieses Stück Boilerplatercode ist auch unter TCFTC (try catch finally try catch) bekannt und bekanntes (Anti)Pattern bei JDBC.

Gruß
Ollie