Is there a better way to do this?

[ChrisMartino]

So, did u do what i said? fix the error, then we can continue.

 

-cb-

 

 

 

also, is that _ALL_ you have in that db table?

 

Yea i fixed that other error, And cleaned the table except from the blank one with a port in.

[ChemicalBliss]

ok instead of going back and forth im goingto offer you some personal help:

I will PM you with details, and hopefully the next post i make will be what we have put together at the end.

 

-cb-

[dirkers]

ok, I tweaked the query to my earlier posting so it finds the lowest available port number. As for "method of comparing time" cb refers to, I don't think that my code omitted any functionality (read the SQL carefully).

 

<?php

function CreateNewGameServer($game, $slots, $days, $owner, $server)
{

$CleanGame		= mysql_real_escape_string($game);
$CleanSlots		= mysql_real_escape_string($slots);
$CleanDays		= mysql_real_escape_string($days);
$CleanOwner		= mysql_real_escape_string($owner);
$CleanServer	= mysql_real_escape_string($server);

$FindServer = mysql_query("SELECT * FROM Servers WHERE ServerName = '".$CleanServer."'");

if (mysql_num_rows($FindServer) == 1) {

	// Check that the clients on the box are under 20
	if (GetClientsOnServer($CleanServer) >= 20) {

		echo"<p>Error, The specified server is currently full.</p>";

	} else {

		// create server entry
		$result = mysql_query("INSERT INTO GameServers (Owner, Game, Slots, Expire, OnServer, DatePurchased) VALUES('".$CleanOwner."', '".$CleanGame."', '".$CleanSlots."', DATE_ADD(NOW, INTERVAL '.$CleanDays.' DAYS), '".$CleanServer."', NOW())");

		// get the new ID that was generated.
		$ServerID = mysql_insert_id();

		// find lowest port such that port+1 has no entry in `GameServers`
		$FindPort = mysql_query("SELECT (Port + 1) AS NewPort FROM `GameServers` WHERE (Port + 1) NOT IN (SELECT Port FROM `GameServers`) ORDER BY NewPort ASC LIMIT 1");
		$Port = mysql_result($FindPort, 0);

		// update new server 
		$Result = mysql_query("UPDATE  `chrismar_xhost`.`GameServers` 
								SET  `Executable` =  'server".$ServerID."',
									`Directory` =  '/home/server".$ServerID."',
									`Port` =  '".$Port."'
								WHERE  `GameServers`.`ServerID` = '".$ServerID."' LIMIT 1 ");

		echo($Port);
	}

} else {
	echo"<p>Invalid server specified.</p>";
}
}

?>

[ChemicalBliss]

This is the solution:

 

I used a strict type comparison operator: !==. Should of been a !=.

 

<?php

function CreateNewGameServer($game, $slots, $days, $owner, $server)
{
$CleanGame = mysql_real_escape_string($game);
$CleanSlots = mysql_real_escape_string($slots);
$CleanDays = mysql_real_escape_string($days);
$CleanOwner = mysql_real_escape_string($owner);
$CleanServer = mysql_real_escape_string($server);

$FindServer = mysql_query("SELECT * FROM Servers WHERE ServerName = '".$CleanServer."'");

if(mysql_num_rows($FindServer) == 1)
{
	// Grammer 
	// Check that the clients on the box are under 20
	if(GetClientsOnServer($CleanServer) > 20)
	{
		echo"<p>Error, The specified server is currently full.</p>";
	}
	else
	{
		// Timestamps please
		$ExpireDate = time()+($CleanDays * 86400); // Add nummber of days to timestamp.
		$DatePurchased = time(); // Much cleaner 

		// Guess you make the server entry now.
		$result = mysql_query("INSERT INTO GameServers (Owner, Game, Slots, Expire, OnServer, DatePurchased) VALUES('".$CleanOwner."', '".$CleanGame."', '".$CleanSlots."', '".$ExpireDate."', '".$CleanServer."', '".$DatePurchased."')");
		$ServerID = mysql_insert_id(); // Set the new ID that was generated.

		// Find New Port
		$FindPort = mysql_query("SELECT Port FROM `GameServers` ORDER BY `GameServers`.`Port`") or die(mysql_error()); // Ascending, no limit

		// Find use an unsused port
		$Port = 0;
		while($row = mysql_fetch_assoc($FindPort)){ // Keep going for every row

			// If Port is 0, set it to the current port number. Otherwise keep it the same.
			$Port = ($Port == 0)? $row['Port'] : $Port;

			// DEBUG
			echo("Port: ".$Port."<Br />");

			// If The port numbers dont match, we got a new one..
			if($row['Port'] != $Port){
				Break; // Break out of the loop, $Port will contain the current unused port number.
			}

			// Increment port, so we can check if the next number is taken in the next loop.
			$Port++;
		}
		// One, Single, Epic (well not so much) Query.
		$query = "UPDATE  `chrismar_xhost`.`GameServers` 
			SET  `Executable` =  'server".$ServerID."',
				`Directory` =  '/home/server".$ServerID."',
				`Port` =  '".$Port."'
			WHERE  `GameServers`.`ServerID` = '".$ServerID."' LIMIT 1 ";
		$Result = mysql_query($query) or die(mysql_error());

		echo("affected rows: ".mysql_affected_rows($Result)."<br/>".$query); // Check the query
	}	

}
else
{
	echo"<p>Invalid server specified.</p>";
}
}

?>

 

This fixes the port problem.

 

Those are some real nice mysql queries there, but its much faster and easier to use timestamps, easier on both ides, and faster on both sides.

 

-cb-

[dirkers]

So you like them fancy queries? Well hold on to your hat cuz we're gonna make it a bit trickier still.

 

The problem with the solution(s) thus far is that there is a race condition in the function. Any number of processes that are kicked off near simultaneously might retrieve the same listing of game servers and consequently come up with the same port number to assign. This is undesired behavior and needs to be remedied.

 

Whether you handle it programmatically or in a query, figuring out which port to use next and to assign it to the newly created game server entry has to happen in an atomic transaction. Now, there are a number of ways you can do this, such as using transactions with the InnoDB storage engine. However, let's keep going with the fancy queries because we can. :-)

 

Start by creating a trigger by executing the following SQL against the DB that contains the tables in question:

 

CREATE TRIGGER find_port BEFORE INSERT
ON GameServers
FOR EACH ROW
BEGIN
        SET NEW.Port = (SELECT (Port + 1) AS Port
                                        FROM GameServers
                                        WHERE (Port + 1) NOT IN (SELECT Port FROM GameServers) ORDER BY Port ASC LIMIT 1);
END

 

This will cause the port to be assigned automatically each time you insert a new entry into the GameServer table. Nice, but it doesn't yet fix our race condition. To do that we need to lock the table for writing while we add the new entry and assign the port. We can do that by issuing a LOCK TABLE statement, which we later undo with UNLOCK TABLES.

 

<?php

function CreateNewGameServer($game, $slots, $days, $owner, $server)
{

$CleanGame	= mysql_real_escape_string($game);
$CleanSlots	= mysql_real_escape_string($slots);
$CleanDays	= mysql_real_escape_string($days);
$CleanOwner	= mysql_real_escape_string($owner);
$CleanServer	= mysql_real_escape_string($server);

$FindServer = mysql_query("SELECT * FROM Servers WHERE ServerName = '".$CleanServer."'");

if (mysql_num_rows($FindServer) == 1) {

	// Check that the clients on the box are under 20
	if (GetClientsOnServer($CleanServer) >= 20) {

		echo"<p>Error, The specified server is currently full.</p>";

	} else {

		// lock tables to fix race condition
		mysql_query("LOCK TABLE GameServer WRITE");

		// create server entry
		$result = mysql_query("INSERT INTO GameServers (Owner, Game, Slots, Expire, OnServer, DatePurchased) VALUES('".$CleanOwner."', '".$CleanGame."', '".$CleanSlots."', DATE_ADD(NOW, INTERVAL '.$CleanDays.' DAYS), '".$CleanServer."', NOW())");

		// release table lock
		mysql_query("UNLOCK TABLES");

		// get the new ID that was generated.
		$ServerID = mysql_insert_id();

		// find lowest port such that port+1 has no entry in `GameServers`
		// and update new server 
		$Result = mysql_query("UPDATE  `chrismar_xhost`.`GameServers` 
						SET  `Executable` =  'server".$ServerID."',
							`Directory` =  '/home/server".$ServerID."',
						WHERE  `GameServers`.`ServerID` = '".$ServerID."' LIMIT 1 ");
	}

} else {
	echo"<p>Invalid server specified.</p>";
}
}

?>

 

For the above code, one should use the mysql_connect() function instead of mysql_pconnect() so as to avoid locked tables if the code gets interrupted before it can execute the UNLOCK TABLES statement.

 

As for performance, yes, it would be better for forgo the queries, but what's the fun in that? But seriously, this is not a high-volume function. If this function ever affects performance, it's because Chris is selling hundred of thousands of servers a day. By then, he can afford to throw a little hardware at the problem.

 

-D