Backup / Restore

[Grok]

If you have ssh, it's not considered shared hosting anymore. -- unless you have a souped up plan --
I repeat... shared hosting plans (most, if not all) will not be able to create cronjobs.

Bluehost offers ssh access on their basic shared hosting plans.
Ipower has a function in the vdeck to add cron jobs on their basic plans.

[Highway of Life]

Hmm... I admin I'm wrong there... I was with iPower for three years, and now that you mention it, I do remember crons with some of their plans.
Though not all hosts have those options, point remaining...

[DavidMJ]

A while back, I had requested an option to use extended and/or complete inserts as a backup option, but the request was rejected... I now create manual backup files from Navicat that have complete and extended inserts, and am able to retain much larger backups due to both those options being available.

I have a bug report on that… its set to review later because it is an issue with IE and forums anywhere, including localhost.
It also makes a huge difference in the size of the backup.

*sigh*
What people tend to forget is that phpBB supports databases outside of MySQL... You may not have used them, you may not have heard of them, you may feel that they are not worth supporting... However, we do.

Extended inserts only, that is to say only or even only, work on MySQL. End of story (not quite end of story, PostgreSQL 8.2 supports them). There are so-called "optimal" insertion formats for ALL databases. Using the same logic, I should tune the database ACP to only work with PostgreSQL's COPY syntax or MSSQL's BULK INSERT syntax.

As to feelings of the backup/restore system being inadequate, I ask you the following: point me in the direction of a better backup and restore system that is integrated with another, fully featured product on the same scale as phpBB that supports all the databases that phpBB supports. I say this because there is simply a finite amount of resources that can be spent on any one particular feature (time, energy, and sometimes my patience with Firebird).

Finally, if you feel that a certain feature is inadequate, then by all means give me a patch and I would be more than happy to review it.

[Handyman]

A while back, I had requested an option to use extended and/or complete inserts as a backup option, but the request was rejected... I now create manual backup files from Navicat that have complete and extended inserts, and am able to retain much larger backups due to both those options being available.

I have a bug report on that… its set to review later because it is an issue with IE and forums anywhere, including localhost.
It also makes a huge difference in the size of the backup.

*sigh*
What people tend to forget is that phpBB supports databases outside of MySQL... You may not have used them, you may not have heard of them, you may feel that they are not worth supporting... However, we do.

Extended inserts only, that is to say only or even only, work on MySQL. End of story (not quite end of story, PostgreSQL 8.2 supports them). There are so-called "optimal" insertion formats for ALL databases. Using the same logic, I should tune the database ACP to only work with PostgreSQL's COPY syntax or MSSQL's BULK INSERT syntax.

As to feelings of the backup/restore system being inadequate, I ask you the following: point me in the direction of a better backup and restore system that is integrated with another, fully featured product on the same scale as phpBB that supports all the databases that phpBB supports. I say this because there is simply a finite amount of resources that can be spent on any one particular feature (time, energy, and sometimes my patience with Firebird).

Finally, if you feel that a certain feature is inadequate, then by all means give me a patch and I would be more than happy to review it.

Thanks David, that sheds a lot of light on the situation.
I completely understand the need to support multiple database types and I really like the options… I just didn't know thats what was holding up the backups.
I have some code sitting on my hard drive that will enable much larger restores, so I'll integrate it and send it to you when I get some time. (not this week)

[code reader]

Finally, if you feel that a certain feature is inadequate, then by all means give me a patch and I would be more than happy to review it. Very Happy

i am not sure if you are really serious in this last sentence. to the best of my memory, this idea (community supplied patches) were scoffed upon in the past.
but if you are, i think i am willing to rise to the challenge, and write the little piece of code that will implement the idea i outlined earlier in this thread describing how a piecemeal restore should work IMO.

just say the word.

[EDIT: it seems that handyman beat me, but my offer still stands: if you are serious about considering community supplied patches, i will try and supply one ]

[DavidMJ]

Finally, if you feel that a certain feature is inadequate, then by all means give me a patch and I would be more than happy to review it. Very Happy

i am not sure if you are really serious in this last sentence. to the best of my memory, this idea (community supplied patches) were scoffed upon in the past.
but if you are, i think i am willing to rise to the challenge, and write the little piece of code that will implement the idea i outlined earlier in this thread describing how a piecemeal restore should work IMO.

just say the word.

[EDIT: it seems that handyman beat me, but my offer still stands: if you are serious about considering community supplied patches, i will try and supply one ]

This is one feature I am willing to bend on, what I said was not an invitation to submit patches for any feature you feel is inadequate.

[code reader]

ok, i accepted the challenge and here is the patch.
some points:
1) it is tested in a very rudimentary level.
this may seem like a serious drawback, but the reality is that the existing code is also tested in a very rudimentary level, so i don't think this is significant.

2) error handling is less than what it should be
again, even though it is very weak, it is still better than the non-existent error handling of the existing code...

3) i added an option for "zip". this is in the restore only. if zip support is not desired, it should be removed, and the patch become a bit simpler (because the zip format is both for compression and packaging multiple files in one zip, there are some additional steps when handling this format)

here is the patch:

Code: Select all

Index: includes/acp/acp_database.php
===================================================================
RCS file: /cvsroot/phpbb/phpBB2/includes/acp/acp_database.php,v
retrieving revision 1.61
diff -u -u -r1.61 acp_database.php
--- includes/acp/acp_database.php	13 Feb 2007 00:14:38 -0000	1.61
+++ includes/acp/acp_database.php	13 Feb 2007 16:16:42 -0000
@@ -1097,7 +1097,8 @@
 			break;
 
 			case 'restore':
-
+				define('CHUNK_SIZE', 0x100000);
+				set_time_limit(0);
 				$this->page_title = 'ACP_RESTORE';
 
 				switch ($action)
@@ -1106,7 +1107,7 @@
 						$delete = request_var('delete', '');
 						$file = request_var('file', '');
 
-						preg_match('#^backup_\d{10,}_[a-z\d]{16}\.(sql(?:\.(?:gz|bz2))?)$#', $file, $matches);
+						preg_match('#^backup_\d{10,}_[a-z\d]{16}\.(sql(?:\.(?:gz|bz2|zip))?)$#', $file, $matches);
 						$file_name = $phpbb_root_path . 'store/' . $matches[0];
 
 						if (!(file_exists($file_name) && is_readable($file_name)))
@@ -1120,96 +1121,132 @@
 							trigger_error($user->lang['BACKUP_DELETE'] . adm_back_link($this->u_action));
 						}
 
-						$data = file_get_contents($file_name);
-
+						$handle = false;
 						switch ($matches[1])
 						{
 							case 'sql.bz2':
-								$data = bzdecompress($data);
+								$handle = bzopen($file_name, 'r');
+								$readfunc = 'bzread';
+								$closefunc = 'bzclose';
 							break;
 							case 'sql.gz':
-								$data = gzinflate(substr($data, 10));
+								$handle = gzopen($file_name, 'r');
+								$readfunc = 'gzread';
+								$closefunc = 'gzclose';
+							break;
+							case 'sql.zip':
+								$zip = zip_open($file_name);
+								if (is_resource($zip))
+								{
+									$entry = $zip_read($zip); 
+									// we use the first entry in the zip file, regardless of its name.
+									if (is_resource($entry))
+									{
+										$handle = zip_entry_open($zip, $entry);
+									}
+								}
+								$readfunc = 'zip_entry_read';
+								$closefunc = 'zip_entry_close';
+							break;
+							default: 
+								$handle = fopen($file_name, 'rb');
+								$readfunc = 'fread';
+								$closefunc = 'fclose';
+							break;
+						}
+						if (!is_resource($handle))
+						{
+							// note: i do not know the right way to handle error here. 
+							// note2: RESTORE_FAILURE is not defined in the lang file.
+							trigger_error($user->lang['RESTORE_FAILURE']);
 							break;
 						}
-
 						$download = request_var('download', '');
 
 						if ($download)
 						{
 							$name = $matches[0];
-
-							switch ($matches[1])
-							{
-								case 'sql':
-									$mimetype = 'text/x-sql';
-								break;
-								case 'sql.bz2':
-									$mimetype = 'application/x-bzip2';
-								break;
-								case 'sql.gz':
-									$mimetype = 'application/x-gzip';
-								break;
-							}
-
 							header('Pragma: no-cache');
-							header("Content-Type: $mimetype; name=\"$name\"");
+							header("Content-Type: text/x-sql; name=\"$name\"");
 							header("Content-disposition: attachment; filename=$name");
-							echo $data;
+							while (true) 
+							{
+								$chunk = $readfunc($handle, CHUNK_SIZE);
+								if (empty($chunk))
+								{
+									$closefunc($handle);
+									break;
+								}
+								echo $chunk;
+							}
 							die;
 						}
 
-						if (!empty($data))
+						if (is_resource($handle) && !empty($readfunc))
 						{
-							// Strip out sql comments...
-							remove_remarks($data);
-
-							// SQLite gets improved performance when you shove all of these disk write queries at once :D
-							if ($db->sql_layer == 'sqlite')
+							switch ($db->sql_layer)
 							{
-								$db->sql_query($data);
+								case 'firebird':
+									$delim = ';;';
+								break;
+								case 'sqlite': // NOTE: i don't really know what is the delimiter for sqlite.
+								case 'mysql':
+								case 'mysql4':
+								case 'mysqli':
+								case 'postgres':
+									$delim = ';';
+								break;
+
+								case 'oracle':
+									$delim = '/';
+								break;
+
+								case 'mssql':
+								case 'mssql_odbc':
+									$delim = 'GO';
+								break;
 							}
-							else
+							$remainder = '';
+							while (true) 
 							{
-								switch ($db->sql_layer)
+								$chunk = $readfunc($handle, CHUNK_SIZE);
+								if (empty($chunk)) 
 								{
-									case 'firebird':
-										$delim = ';;';
-									break;
-
-									case 'mysql':
-									case 'mysql4':
-									case 'mysqli':
-									case 'postgres':
-										$delim = ';';
-									break;
-
-									case 'oracle':
-										$delim = '/';
-									break;
-
-									case 'mssql':
-									case 'mssql_odbc':
-										$delim = 'GO';
+								// there is no good EOF function that applies to all possibilities, 
+								// so we use the next option: when the read operation returned an empty string.
 									break;
 								}
-								$pieces = split_sql_file($data, $delim);
-
-								$sql_count = count($pieces);
-								for ($i = 0; $i < $sql_count; $i++)
+								remove_remarks($chunk);
+								if (empty($chunk))
 								{
-									$sql = trim($pieces[$i]);
-
-									if (!empty($sql) && $sql[0] != '#')
+									continue;
+								}
+								$pieces = split_sql_file($chunk, $delim);
+								if (empty($pieces))
+								{
+									continue;
+								}
+								$pieces[0] = $reminder . $pieces[0];
+								$remainder = array_pop($pieces);
+								if (preg_match("/{$delim}$/", $remainder)) // == if reaminder is a full query
+								{
+									$pieces[] = $remainder;
+									$remainder = '';
+								}
+								foreach ($pieces as $sql)
+								{
+									remove_remarks($sql);
+									if (!empty($sql))
 									{
 										$db->sql_query($sql);
 									}
 								}
 							}
+							$closefunc($handle);
 						}
 						add_log('admin', 'LOG_DB_RESTORE');
 						trigger_error($user->lang['RESTORE_SUCCESS'] . adm_back_link($this->u_action));

[DavidMJ]

$zip_read is not defined, ever. It is just wantonly used. Outside of that, the entire approach to zip is just bad. It takes many assumptions and does not even consider the fact that we have a fully working zip class written in PHP and available to every installation... $reminder? What's this? Never defined... There are various other issues (like your method of chunking the file)

There is no way that code will be replaced in Olympus with untested code. The backup system has significantly matured over releases, much of it's code is known to be good.

[code reader]

$zip_read is not defined, ever.

a typo. should have been zip_read (no $).

Outside of that, the entire approach to zip is just bad. It takes many assumptions and does not even consider the fact that we have a fully working zip class written in PHP and available to every installation

i use a unified approach for flat (uncompressed), zip, bzip and gzip files. i don't believe one could readily do that with the zip class, and i don't see any reason to.
the whole "zip" part was just a whim. since the current code does not support zip for backup, the simplest thing would be to just remove the parts dealing with it in the restore.

$reminder? What's this? Never defined...

that was another typo. it should have been, of course, $remainder

The backup system has significantly matured over releases, much of it's code is known to be good.

i did not touch the backup code.
the restore code is far from being "mature" or "good". the only thing known about it is that it often fails with timeouts. (it doesn't even test for results or handles errors in any way)
the approach of reading the whole backup file into a single variable in memory, and then throwing this variable around creating multiple copies (up to 4 copies of the whole files are created every time), is just a toy. it breaks as soon as the database grows above 1/4 of available physical memory, and on the way to failure, it may choke the host to death.

i did not expect you to take my code "as is" and put it into olympus without any modifications or testing, but it does supply a working example of one way to get the job done.
leaving the existing "restore" part of the code in place means, practically going out without a working backup/restore system.

[DavidMJ]

Alright, the output of the backup system is now optimal for PostgreSQL and MySQL. Many bugs have been fixed as well as some nice speed increases. The emphasis on this backup system has an emphasis on memory savings but it should still be speedy enough. Any code that was not outright rewritten was refactored.