DB.close() needs to be called
Looks like Toby's recent change to ApplicationManager.py causes DB.close() to never be called when you hit Shutdown in the Control Panel. This is a bad thing for the Berkeley storages because their .close() must get called or you'll end up with corrupt databases or worse <wink>. ---------------------------- revision 1.83 date: 2002/11/12 17:19:13; author: htrd; state: Exp; lines: +2 -2 dont close the storages mid-transaction. need to check whether we need to close them at the end of z2.py ---------------------------- "Um, yeah!" would be my answer. :) So here's a patch to z2.py to fix this. I won't check this in because it looks a little ugly to me and I'm not sure what the right fix is, but we definitely need to fix this before Zope 2.6.1 is released. -Barry Index: z2.py =================================================================== RCS file: /cvs-repository/Zope/z2.py,v retrieving revision 1.88 diff -u -r1.88 z2.py --- z2.py 4 Nov 2002 13:33:46 -0000 1.88 +++ z2.py 12 Nov 2002 19:15:51 -0000 @@ -909,5 +909,7 @@ import Lifetime Lifetime.loop() code = sys.ZServerExitCode +import Zope +Zope.DB.close() zLOG.LOG("z2", zLOG.INFO, 'Exiting with code %d' % code ) sys.exit(code)
On Tuesday 12 November 2002 7:16 pm, Barry A. Warsaw wrote:
Looks like Toby's recent change to ApplicationManager.py causes DB.close() to never be called when you hit Shutdown in the Control Panel.
Yes.
This is a bad thing for the Berkeley storages because their .close() must get called or you'll end up with corrupt databases or worse <wink>.
How much of that paragraph is covered by the wink? Even before my recent changes, there are plenty of other ways that Zope can open the storage, run a transaction, then hit an error before properly starting up which means it fails to close the database correctly. Would this be bad?
So here's a patch to z2.py to fix this. I won't check this in because it looks a little ugly to me and I'm not sure what the right fix is,
That is exactly what I was planning to commit. Before making that change, I need to remove a couple other calls to db.close() to prevent it being called twice on the same db.
but we definitely need to fix this before Zope 2.6.1 is released.
2.6.1? This change has only been on the trunk, not the 2.6 maintenance branch.
"TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes:
>> This is a bad thing for the Berkeley storages because their >> .close() must get called or you'll end up with corrupt >> databases or worse <wink>. TD> How much of that paragraph is covered by the wink? The "or worse" part. If you've enable autopacking and you don't cleanly close the storage, you won't exit the process because the autopack thread won't get stopped and joined. We could make the autopack thread a daemon process, but it makes me nervous to do that in the general case, because if you exit while autopack is running, you could corrupt your database. Hmm, I guess we could make it a daemon process and yet still cleanly stop it on a close()... TD> Even before my recent changes, there are plenty of other ways TD> that Zope can open the storage, run a transaction, then hit an TD> error before properly starting up which means it fails to TD> close the database correctly. TD> Would this be bad? It wouldn't be good, but remember that we run BerkeleyDB in auto-recovery mode, so it means that the next time you open the database, it will run recovery (you can also run recovery manually). However, depending on your checkpointing policy, and the size of committed data since your last checkpoint, recovery could end up taking a long time. Sure, any number of bad things can happen at any time, and defense against that is one of the benefits of using BerkeleyDB underneath. But under normal operations, we definitely want to exit cleanly. >> So here's a patch to z2.py to fix this. I won't check this in >> because it looks a little ugly to me and I'm not sure what the >> right fix is, TD> That is exactly what I was planning to commit. Cool. :) TD> Before making that change, I need to remove a couple other TD> calls to db.close() to prevent it being called twice on the TD> same db. Ok. >> but we definitely need to fix this before Zope 2.6.1 is >> released. TD> 2.6.1? This change has only been on the trunk, not the 2.6 TD> maintenance branch. You're right. I was running from the trunk. Thanks, -Barry
On Wednesday 13 November 2002 2:10 pm, Barry A. Warsaw wrote:
"TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes: >> This is a bad thing for the Berkeley storages because their >> .close() must get called or you'll end up with corrupt >> databases or worse <wink>.
TD> How much of that paragraph is covered by the wink?
The "or worse" part. If you've enable autopacking and you don't cleanly close the storage, you won't exit the process because the autopack thread won't get stopped and joined.
Yes, I had the same problem with DirectoryStorage, which uses a seperate thread to move writes from its journal directory into the database directory.
We could make the autopack thread a daemon process
Thats how DirectoryStorage now works.
It wouldn't be good, but remember that we run BerkeleyDB in auto-recovery mode, so it means that the next time you open the database, it will run recovery (you can also run recovery manually). However, depending on your checkpointing policy, and the size of committed data since your last checkpoint, recovery could end up taking a long time.
Last time I looked at your BerkeleyDB storages, the administrator needed to implement his own checkpointing policy. I always thought that was a disadvantage. Would it now be a good idea for the storages to trigger their own checkpointing inside the autopacker thread?
Sure, any number of bad things can happen at any time, and defense against that is one of the benefits of using BerkeleyDB underneath. But under normal operations, we definitely want to exit cleanly.
Agreed, on both points.
Maybe normal shutdown should manually call the shutdown signal handler function and normal restart should manually call the restart signal handler function? On Wed, 2002-11-13 at 10:28, Toby Dickenson wrote:
On Wednesday 13 November 2002 2:10 pm, Barry A. Warsaw wrote:
> "TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes: >> This is a bad thing for the Berkeley storages because their >> .close() must get called or you'll end up with corrupt >> databases or worse <wink>.
TD> How much of that paragraph is covered by the wink?
The "or worse" part. If you've enable autopacking and you don't cleanly close the storage, you won't exit the process because the autopack thread won't get stopped and joined.
Yes, I had the same problem with DirectoryStorage, which uses a seperate thread to move writes from its journal directory into the database directory.
We could make the autopack thread a daemon process
Thats how DirectoryStorage now works.
It wouldn't be good, but remember that we run BerkeleyDB in auto-recovery mode, so it means that the next time you open the database, it will run recovery (you can also run recovery manually). However, depending on your checkpointing policy, and the size of committed data since your last checkpoint, recovery could end up taking a long time.
Last time I looked at your BerkeleyDB storages, the administrator needed to implement his own checkpointing policy. I always thought that was a disadvantage. Would it now be a good idea for the storages to trigger their own checkpointing inside the autopacker thread?
Sure, any number of bad things can happen at any time, and defense against that is one of the benefits of using BerkeleyDB underneath. But under normal operations, we definitely want to exit cleanly.
Agreed, on both points.
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
Nevermind. I see what Toby did in the signals module and it makes sense. On Wed, 2002-11-13 at 11:01, Chris McDonough wrote:
Maybe normal shutdown should manually call the shutdown signal handler function and normal restart should manually call the restart signal handler function?
On Wed, 2002-11-13 at 10:28, Toby Dickenson wrote:
On Wednesday 13 November 2002 2:10 pm, Barry A. Warsaw wrote:
>> "TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes: >> This is a bad thing for the Berkeley storages because their >> .close() must get called or you'll end up with corrupt >> databases or worse <wink>.
TD> How much of that paragraph is covered by the wink?
The "or worse" part. If you've enable autopacking and you don't cleanly close the storage, you won't exit the process because the autopack thread won't get stopped and joined.
Yes, I had the same problem with DirectoryStorage, which uses a seperate thread to move writes from its journal directory into the database directory.
We could make the autopack thread a daemon process
Thats how DirectoryStorage now works.
It wouldn't be good, but remember that we run BerkeleyDB in auto-recovery mode, so it means that the next time you open the database, it will run recovery (you can also run recovery manually). However, depending on your checkpointing policy, and the size of committed data since your last checkpoint, recovery could end up taking a long time.
Last time I looked at your BerkeleyDB storages, the administrator needed to implement his own checkpointing policy. I always thought that was a disadvantage. Would it now be a good idea for the storages to trigger their own checkpointing inside the autopacker thread?
Sure, any number of bad things can happen at any time, and defense against that is one of the benefits of using BerkeleyDB underneath. But under normal operations, we definitely want to exit cleanly.
Agreed, on both points.
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
On Wednesday 13 November 2002 4:01 pm, Chris McDonough wrote:
Maybe normal shutdown should manually call the shutdown signal handler function and normal restart should manually call the restart signal handler function?
We are pretty close to that now, which I agree is a good thing. The shutdown signal handler is now a one-liner that calls Lifetime.shutdown. The ZMI handler is a two-liner that calls the same function, then returns some html. Lifetime.shutdown causes the asyncore select loop to return (possibly after a brief pause to allow clients to disconnect cleanly), and z2.py closes the databases (and hence storages) as its very last action.
"TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes:
>> worse" part. If you've enable autopacking and you don't >> cleanly close the storage, you won't exit the process because >> the autopack thread won't get stopped and joined. TD> Yes, I had the same problem with DirectoryStorage, which uses TD> a seperate thread to move writes from its journal directory TD> into the database directory. Ah. Is this transactional? What would happen if the thread got killed in the middle of the move? >> We could make the >> autopack thread a daemon process TD> Thats how DirectoryStorage now works. Hmm, maybe we make it a daemon thread and put a timeout on the join, so we'll try to exit cleanly and won't hang if we can't. Autopacking should be safe transactionally, but we'd have to do a recovery if it got killed uncleanly. >> It wouldn't be good, but remember that we run BerkeleyDB in >> auto-recovery mode, so it means that the next time you open the >> database, it will run recovery (you can also run recovery >> manually). However, depending on your checkpointing policy, >> and the size of committed data since your last checkpoint, >> recovery could end up taking a long time. TD> Last time I looked at your BerkeleyDB storages, the TD> administrator needed to implement his own checkpointing TD> policy. I always thought that was a disadvantage. Would it now TD> be a good idea for the storages to trigger their own TD> checkpointing inside the autopacker thread? Actually, now you can configure the storages to automatically checkpoint every nth ZODB transaction. Checkpointing in a thread may or may not provide additional benefit. >> Sure, any number of bad things can happen at any time, and >> defense against that is one of the benefits of using BerkeleyDB >> underneath. But under normal operations, we definitely want to >> exit cleanly. TD> Agreed, on both points. Cool. -Barry
(cc zodb-dev, who may also be interested) On Wednesday 13 November 2002 4:18 pm, Barry A. Warsaw wrote:
"TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes: >> worse" part. If you've enable autopacking and you don't >> cleanly close the storage, you won't exit the process because >> the autopack thread won't get stopped and joined.
TD> Yes, I had the same problem with DirectoryStorage, which uses TD> a seperate thread to move writes from its journal directory TD> into the database directory.
Ah. Is this transactional? What would happen if the thread got killed in the middle of the move?
There are lots of ways to look at that question.... I am relying on the filesystem guarantee that the files are all either in their original directory, or their new directory. DirectoryStorage has its equivalent of BerkeleyStorage's autorecovery which, at startup, asynchronously resumes the file moving wherever it left off. A more interesting question is what happens if the thread is killed when it has moved some, but not all, of the files which relate to one transaction. In DirectoryStorage terminology this leaves the data directory in a state which "is not a snapshot". The only disadvantage is that you can not use the tools which assume the data directory is a snapshot: the checking tool, the incremental back tool, and the replication tool.
>> We could make the >> autopack thread a daemon process
TD> Thats how DirectoryStorage now works.
Hmm, maybe we make it a daemon thread and put a timeout on the join, so we'll try to exit cleanly and won't hang if we can't.
Because some of the operations performed in the daemon thread take a long time to complete? Would it be possible to break those operations into transactionally-coherent chunks which complete in a reasonable time? close() could wait for the current chunk to finish.
Autopacking should be safe transactionally, but we'd have to do a recovery if it got killed uncleanly.
Doesnt having a good checkpointing strategy mean that this should never take long?
TD> Last time I looked at your BerkeleyDB storages, the TD> administrator needed to implement his own checkpointing TD> policy. I always thought that was a disadvantage. Would it now TD> be a good idea for the storages to trigger their own TD> checkpointing inside the autopacker thread?
Actually, now you can configure the storages to automatically checkpoint every nth ZODB transaction. Checkpointing in a thread may or may not provide additional benefit.
Interesting. BerkeleyStorage's automatic checkpointing is currently triggered inside a commit or abort. This means the checkpoint overhead is incurred at the one time you dont want it - while a user is waiting for his transaction to commit. DirectoryStorage's main use for the thread is to perform its equivalent asynchronously. Assuming your storage is not saturated with writes (which only happens in benchmarks ;-) then checkpointing happens for free.
"TD" == Toby Dickenson <tdickenson@geminidataloggers.com> writes:
>> What would happen if the thread got killed in the middle of the >> move? TD> A more interesting question is what happens if the thread is TD> killed when it has moved some, but not all, of the files which TD> relate to one transaction. That's more what I was thinking about. TD> In DirectoryStorage terminology this leaves the data directory TD> in a state which "is not a snapshot". The only disadvantage is TD> that you can not use the tools which assume the data directory TD> is a snapshot: the checking tool, the incremental back tool, TD> and the replication tool. Interesting. Berkeley storage has something similar. Let's say the storage crashes in the middle of tpc_vote() or tpc_finish(). First, BerkeleyDB's recovery should clean up any of its corrupt transactions. Then the storage has its own recovery process for the "current" transaction, i.e. the one we were in the middle of at the time of the crash. Depending on when the crash occurred, the storages will either commit or abort the current transaction (if the crash happened in tpc_finish(), we'll complete the commit, otherwise we abort the transaction). So I believe -- although this isn't battle tested -- that the Berkeley storages should be pretty safe against inconsistency and corruption. >> We could make the autopack thread a daemon process >> TD> Thats how DirectoryStorage now works. Hmm, maybe we make >> it a daemon thread and put a timeout on the join, so we'll try >> to exit cleanly and won't hang if we can't. TD> Because some of the operations performed in the daemon thread TD> take a long time to complete? Potentially yes, although the steps in the pack process are BerkeleyDB transactionally protected. I think the mark-and-sweep phases are examples of things that could take a long time. It should be possible to craft some escape hatches to do a more controlled shutdown. TD> Would it be possible to break those operations into TD> transactionally-coherent chunks which complete in a reasonable TD> time? close() could wait for the current chunk to finish. Yep. >> Autopacking should be safe transactionally, but we'd have to do >> a recovery if it got killed uncleanly. TD> Doesnt having a good checkpointing strategy mean that this TD> should never take long? I think checkpointing itself is a trade-off between the length of time it takes to checkpoint and the length of time it could potentially take to recover. >> TD> Last time I looked at your BerkeleyDB storages, the TD> >> administrator needed to implement his own checkpointing TD> >> policy. I always thought that was a disadvantage. Would it now >> TD> be a good idea for the storages to trigger their own TD> >> checkpointing inside the autopacker thread? >> Actually, now you can configure the storages to automatically >> checkpoint every nth ZODB transaction. Checkpointing in a >> thread may or may not provide additional benefit. TD> Interesting. BerkeleyStorage's automatic checkpointing is TD> currently triggered inside a commit or abort. This means the TD> checkpoint overhead is incurred at the one time you dont want TD> it - while a user is waiting for his transaction to TD> commit. DirectoryStorage's main use for the thread is to TD> perform its equivalent asynchronously. Assuming your storage TD> is not saturated with writes (which only happens in benchmarks TD> ;-) then checkpointing happens for free. That's a very interesting idea! Hmm, checkpoint in a thread. Yeah, that sounds much better, although I'm mildly concerned with concurrancy issues, especially given that we don't use the BerkeleyDB locking subsystem. I'll have to read up on the docs, but it's a good idea. -Barry
On Wednesday 13 November 2002 5:27 pm, Barry A. Warsaw wrote:
Potentially yes, although the steps in the pack process are BerkeleyDB transactionally protected. I think the mark-and-sweep phases are examples of things that could take a long time. It should be possible to craft some escape hatches to do a more controlled shutdown.
Yes, DirectoryStorage's referential integrity rules had to be carefully crafted to ensure that it can bomb out of a pack at short notice.
Working on updating my ZOPE and ZEO RPMs I got to wondering... What's in the default data.fs that ships with Zope? I mean, ZEO (actually ZODB) auto-creates a data.fs when one isn't found, so why does Zope come with one? Or if there -is- something Zope-specific in data.fs, then shouldn't there be a warning in the ZEO notes that when ZEO is used _underneath_ Zope, be sure to copy the data.fs that comes with it? My experience has always been with ZEO and StandaloneZODB, not ZEO+Zope so I'm puzzled. -Jeff
It is only there due to lack of time to take it out. We had planned to take it out for 2.6, but time was never made to replace it with code to bootstrap an empty storage with the proper root level elements still residing in Data.fs.in. -Casey On Wednesday 13 November 2002 02:22 pm, Jeff Rush wrote:
Working on updating my ZOPE and ZEO RPMs I got to wondering...
What's in the default data.fs that ships with Zope? I mean, ZEO (actually ZODB) auto-creates a data.fs when one isn't found, so why does Zope come with one?
Or if there -is- something Zope-specific in data.fs, then shouldn't there be a warning in the ZEO notes that when ZEO is used _underneath_ Zope, be sure to copy the data.fs that comes with it?
My experience has always been with ZEO and StandaloneZODB, not ZEO+Zope so I'm puzzled.
-Jeff
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
I don't quite understand -- so there *are* root level elements specific to Zope that need to be copied into a Zope-over-ZEO environment? (hm, how do those elements get into a non-FileStorage Zope-over-ZEO environment?) And do those elements interfere even a little in a non-Zope-just-ZEO environment? The only way I can imagine, other than simplistic name clashes would be if a full iteration of such a ZODB would cause unghosting of objects lacking Zope .pyc and raise unnecessary exceptions. I ask because I'm trying to decide whether two ZEO RPMs are needed re ZEO-wo-Zope-2.0-1.i386.rpm and ZEO-w-Zope-2.0-1.i386.rpm, or just one. Somewhat similar to how the Zope RPMs have separate ZServer and PCGI flavor packages. -Jeff Rush Casey Duncan wrote:
It is only there due to lack of time to take it out. We had planned to take it out for 2.6, but time was never made to replace it with code to bootstrap an empty storage with the proper root level elements still residing in Data.fs.in.
-Casey
On Wednesday 13 November 2002 02:22 pm, Jeff Rush wrote:
Working on updating my ZOPE and ZEO RPMs I got to wondering...
What's in the default data.fs that ships with Zope? I mean, ZEO (actually ZODB) auto-creates a data.fs when one isn't found, so why does Zope come with one?
Or if there -is- something Zope-specific in data.fs, then shouldn't there be a warning in the ZEO notes that when ZEO is used _underneath_ Zope, be sure to copy the data.fs that comes with it?
My experience has always been with ZEO and StandaloneZODB, not ZEO+Zope so I'm puzzled.
-Jeff
Jeff Rush wrote:
I don't quite understand -- so there *are* root level elements specific to Zope that need to be copied into a Zope-over-ZEO environment? (hm, how do those elements get into a non-FileStorage Zope-over-ZEO environment?)
And do those elements interfere even a little in a non-Zope-just-ZEO environment? The only way I can imagine, other than simplistic name clashes would be if a full iteration of such a ZODB would cause unghosting of objects lacking Zope .pyc and raise unnecessary exceptions.
I ask because I'm trying to decide whether two ZEO RPMs are needed re ZEO-wo-Zope-2.0-1.i386.rpm and ZEO-w-Zope-2.0-1.i386.rpm, or just one. Somewhat similar to how the Zope RPMs have separate ZServer and PCGI flavor packages.
Data.fs.in contains nothing but the examples folder, if I remember correctly. Zope has no trouble starting without Data.fs.in. Just ignore it. ;-) Can we discuss the package separation, BTW? It doesn't currently feel optimal. There really shouldn't be a ZServer package, since ZServer is bound tightly with Zope2. I would want to see the following packages: ZODB3 ZEO2 Zope2 Zope2-pcgi The both the Zope2 and ZEO2 packages should depend on the ZODB3 package and the system python package (which must be Python 2.1.x, not Python 2.2.x). The Zope2-pcgi package should depend on the Zope2 package. Some suggested add-ons: Zope2-CMF Zope2-Plone Zope2-Squishdot Shane
Whilst we are on the subject, I now ship the Plone Windows Installation (and I think the Mac) without a ZODB, it monkey patches OFS.Application.initialize to add in the Plone stuff on start up. This lets us do some neat stuff and make a smaller download. Im not sure how zope 3 does it, but I was hoping to make the scripts that get run to initialise a ZODB a zcml parameter so that all I would do alter the zcml file to load up the db.
Zope2-Plone
Is anyone willing to help on this rpm? We only have deb's at the moment. Any help would wonderful ;) -- Andy McKay www.agmweb.ca
Casey Duncan wrote:
It is only there due to lack of time to take it out. We had planned to take it out for 2.6, but time was never made to replace it with code to bootstrap an empty storage with the proper root level elements still residing in Data.fs.in.
IIRC, the proper root level elements are now created in code. However, various unit tests of zope and 3rd party products rely on having a Data.fs.in. -- Steve Alexander
Steve Alexander wrote:
Casey Duncan wrote:
It is only there due to lack of time to take it out. We had planned to take it out for 2.6, but time was never made to replace it with code to bootstrap an empty storage with the proper root level elements still residing in Data.fs.in.
IIRC, the proper root level elements are now created in code. However, various unit tests of zope and 3rd party products rely on having a Data.fs.in.
FYI all dependencies of Zope unit tests on Data.fs* have been removed. See lib/python/Testing/custom_zodb.py, which is much simpler than it used to be. Shane
participants (8)
-
Andy McKay -
barry＠zope.com -
Casey Duncan -
Chris McDonough -
Jeff Rush -
Shane Hathaway -
Steve Alexander -
Toby Dickenson