Backup / Restore

Discussion of general topics related to the new version and its place in the world. Don't discuss new features, report bugs, ask for support, et cetera. Don't use this to spam for other boards or attack those boards!
Forum rules
Discussion of general topics related to the new release and its place in the world. Don't discuss new features, report bugs, ask for support, et cetera. Don't use this to spam for other boards or attack those boards!
User avatar
Grok
Registered User
Posts: 35
Joined: Thu Apr 06, 2006 3:47 pm

Re: Backup / Restore

Post by Grok »

Highway of Life wrote: 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.

Image

User avatar
Highway of Life
Registered User
Posts: 1399
Joined: Tue Feb 08, 2005 10:18 pm
Location: I'd love to change the World, but they won't give me the Source Code
Contact:

Re: Backup / Restore

Post by 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. :oops:
Though not all hosts have those options, point remaining...
Image

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by DavidMJ »

Handyman wrote:
Highway of Life wrote: A while back, I had requested an option to use extended and/or complete inserts as a backup option, but the request was rejected... :x 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. :D
Freedom from fear

User avatar
Handyman
Registered User
Posts: 522
Joined: Thu Feb 03, 2005 5:09 am
Location: Where no man has gone before!
Contact:

Re: Backup / Restore

Post by Handyman »

DavidMJ wrote:
Handyman wrote:
Highway of Life wrote: A while back, I had requested an option to use extended and/or complete inserts as a backup option, but the request was rejected... :x 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. :D

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)
My phpBB3 Mods || My Mod Queue
Search Engine Friendly (SEO) URLs || Profile link on Avatar and/or Username || AJAX Chat
Display Posts Anywhere || CashMod || AJAX Quick Edit || AJAX Quick Reply

Image

code reader
Registered User
Posts: 653
Joined: Wed Sep 21, 2005 3:01 pm

Re: Backup / Restore

Post by code reader »

DavidMJ wrote: 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 ]

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by DavidMJ »

code reader wrote:
DavidMJ wrote: 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.
Freedom from fear

code reader
Registered User
Posts: 653
Joined: Wed Sep 21, 2005 3:01 pm

Re: Backup / Restore

Post by 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));

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by 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.
Freedom from fear

code reader
Registered User
Posts: 653
Joined: Wed Sep 21, 2005 3:01 pm

Re: Backup / Restore

Post by code reader »

DavidMJ wrote: $zip_read is not defined, ever.
a typo. should have been zip_read (no $).
DavidMJ wrote: 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.
DavidMJ wrote: $reminder? What's this? Never defined...
that was another typo. it should have been, of course, $remainder
DavidMJ wrote: 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.

User avatar
DavidMJ
Registered User
Posts: 932
Joined: Thu Jun 16, 2005 1:14 am
Location: Great Neck, NY

Re: Backup / Restore

Post by 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.
Attachments
acp_database.tar
(57 KiB) Downloaded 420 times
Freedom from fear

Post Reply