Singleton with multiple databases

Question

example app, having employee information and being accessed by different applications like payroll and pos. i have employee data in one database, payroll data and pos in separate databases each.

i have a database connection class like below, so everytime i want to get a connection to a db i just do $conn = Database::getInstance(db1).

works great, but is super slow basically. makes the app run really slow. Any tips on why that is so or better yet alternative ideas to do this?

any help will be greatly appreciated

<?php    
class Database {
        private $db;
        static $db_type;
        static $_instance;

        private function __construct($db){
            switch($db) {
                case "db1":
                  try{
                      $this->db = new PDO("mysql:host=" . DB_HOST . ";dbname=" . DB_NAME, DB_USER, DB_PASSWORD);
                  }
                  catch(PDOException $e){
                      print "Error!: " . $e->getMessage() . "<br />";
                      die();
                  }
                break;
                case "db2":
                  try{
                      $this->db = new PDO("mysql:host=" . DB_HOST_2 . ";dbname=" . DB_NAME_2, DB_USER_2, DB_PASSWORD_2);
                  }
                  catch(PDOException $e){
                      print "Error!: " . $e->getMessage() . "<br />";
                      die();
                  }
                break;
            }
            self::$db_type = $db;

        }

        private function __clone(){}

        static function getInstance($db_type){
            if(!(self::$_instance) || $db != self::$db_type){
                self::$_instance = new self($db_type);
            }
            return self::$_instance;
        }
    }
?>

It shouldn't take very long to make a connection to the database, and I wouldn't think your app would be 'slow' overall because you have different DBs to connect to. Are the databases local? or on a remote server? — Dan
– Dan, Commented Nov 2, 2012 at 1:09
databases are all local. i am currently using this approach, and its taking quite a long time to load, as compared to when i have everything in just one db — Mohamed Omar
– Mohamed Omar, Commented Nov 2, 2012 at 1:11
You can help find the specific line slowing things down by capturing microtime() before after certain lines. If might help if you can specifically isolate your issue to the line connecting to the database. — Dan
– Dan, Commented Nov 2, 2012 at 1:18

zeal · Accepted Answer · 2012-11-02 01:16:44Z

1

With this design. If you change databases then it destroys the connection to the previous database.

Make separate objects for each connection then switch between the connection objects.

Also, this is not thread safe for the same reason. If multiple functions are hitting this as the same time, one can disconnect the other before its done loading.

You really should just create a new connection object for each function and not share it between functions or other objects.

answered Nov 2, 2012 at 1:16

zeal

7401 gold badge4 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Mohamed Omar Over a year ago

Awesome! Changed it to an array holding the objects instead, works pefectly!

Mohamed Omar Over a year ago

for others for reference:

static function getInstance($db_type = ""){ 				if(!isset(self::$connections[$db_type])){ 			self::$connections[$db_type] = new self($db); 		} 		return self::$connections[$db_type]; 	}

Mohamed Omar Over a year ago

@johnny man I am so sorry, been almost 3 years. I cant even remember haha. I think what I posted just above your comment, but to be honest I cant even recognize that! Been a while.

johnny Over a year ago

@user1239663 no worries. I forget how to w old some answers are. Thanks.

dmarra · Accepted Answer · 2012-11-02 01:16:19Z

1

Do not create new object constantly. What is happening is that everytime you request another database type, you are recreating it via the new keyword (although hard to confirm without seeing code that uses this).

$_instance is a static member, so you are constantly overwriting it when you change database type. so is $db_type for that matter

While this is overkill for what you are doing (why not just have two variables for each DB?), you could try something more like this:

<?php    
class Database {
        private $db;
        static $db_types;


        private function __construct($db){
            switch($db) {
                case "db1":
                  try{
                      $db_types[$db] = new PDO("mysql:host=" . DB_HOST . ";dbname=" . DB_NAME, DB_USER, DB_PASSWORD);
                  }
                  catch(PDOException $e){
                      print "Error!: " . $e->getMessage() . "<br />";
                      die();
                  }
                break;
                case "db2":
                  try{
                      $db_types[$db] = new PDO("mysql:host=" . DB_HOST_2 . ";dbname=" . DB_NAME_2, DB_USER_2, DB_PASSWORD_2);
                  }
                  catch(PDOException $e){
                      print "Error!: " . $e->getMessage() . "<br />";
                      die();
                  }
                break;
            }


        }

        private function __clone(){}

        static function getInstance($db_type){
            if(!inarray($db_types[$db_type]){
                $db_types[$db_type] = new self($db_type);
            }
            return $db_types[$db_type];
        }
    }
?>

NOTE: syntax is likely off. Just wanted to demonstrate the pattern.

answered Nov 2, 2012 at 1:16

dmarra

8739 silver badges23 bronze badges

2 Comments

Mohamed Omar Over a year ago

let me try this out and will let you know.

Mohamed Omar Over a year ago

managed to get this to work, but didnt sort the speed issue. still takes quite sometime to load. i guess maybe something else is causing this issue, but i'll try the solution by prodigitalson to compare then i'll know

prodigitalson · Accepted Answer · 2012-11-02 01:28:44Z

1

I dont see why that would be making things slow other than the fact that youre constantly switching conncections. The only thing i can suggest here is to allow multiple connections instead of switching them:

class Database {
   protected static $connections;

   protected $activeConnections = array();

   protected static $instance;

   protected function __construct() {

   }

   public static loadConnections(array $connections) {

      self::$connections = $connections;
   }

   public function getConnection($name)
   {
      if(!isset($this->activeConnections[$name]) {
          if(!isset(self::$connections[$name]) {
             throw new Exception('Connection "' . $name . '" is not configured.');
          }

           $this->activeConnections[$name] = new PDO(
              self::$connections[$name]['dsn'],
              self::$connections[$name]['username'], 
              self::$connections[$name]['password']
          ); 

      }

      return $this->activeConnections[$name];
   }
}

// usage

Database::loadConnections(array(
   'db1' => array(
       'dsn' => "mysql:host=" . DB_HOST . ";dbname=" . DB_NAME,
       'user' => DB_USER,
       'password' => DB_PASSWORD,
    ),
    'db2' => array(
       'dsn' => "mysql:host=" . DB_HOST2 . ";dbname=" . DB_NAME2,
       'user' => DB_USER2,
       'password' => DB_PASSWORD2,
)));

$conn1 = Database::getInstance()->getConnection('db1');
$conn2 = Database::getInstance()->getConnection('db2');

Using something like this you can actually manange several open connections at a time, and they are lazy loaded - meaning you dont actually instantiate a PDO connection until you ask for it with Database::getConnection Likewise you can inject additional DSN's and credentials in at anytime. Personally i would load thes form configuration right on to the class instead of hard coding them with constants int he class. then you could so something like:

// gives us an array
$config = Config::load('path/to/db/config.yml');

Database::loadConnections($config);

answered Nov 2, 2012 at 1:28

prodigitalson

60.6k10 gold badges106 silver badges118 bronze badges

3 Comments

Mohamed Omar Over a year ago

let me this try out and will let you know how it goes

Mohamed Omar Over a year ago

sorry, is this missing a static function getInstance() ?

prodigitalson Over a year ago

Its not a full class... i just outlined the differences with yours :-)

iWantSimpleLife · Accepted Answer · 2012-11-02 01:14:31Z

0

How about changing it to also use lazy load. You do not need to connect to the databases in the contractor. Only connect when the database is first required. That way, if the page only uses one of the connection, it does not need to wait for the other databases.

answered Nov 2, 2012 at 1:14

iWantSimpleLife

1,96414 silver badges22 bronze badges

1 Comment

Mohamed Omar Over a year ago

sorry not sure what you mean, could you please explain a bit more

Dan · Accepted Answer · 2012-11-02 01:20:22Z

0

Check the value of DB_HOST and DB_HOST_2. Previously I've found MySQL extremely slow to connect using "127.0.0.1", but connecting instantly using "localhost".

It depends on how your server is setup, but just thought it might help.

answered Nov 2, 2012 at 1:20

Dan

2,0933 gold badges22 silver badges31 bronze badges

Collectives™ on Stack Overflow

Singleton with multiple databases

5 Answers 5

4 Comments

2 Comments

3 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

4 Comments

2 Comments

3 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related