search

IslandzVW / halcyon

InWorldz Halcyon 3d virtual reality world simulator
BSD 3-Clause "New" or "Revised" License
21 stars 26 forks source link

Allow StoreAsset to be optionally async, default disabled (opt-in). #413

Closed appurist closed 6 years ago

appurist commented 6 years ago

Due to subsequent fetches coming only from Aperture (which is unaware of the new asset until the StoreAsset completes the write to CF), we can't signal the session to continue by returning early (as soon as the asset is in the region server cache).

This change effectively reverts the change to make StoreAsset asynchronous, although it leaves it optional in the Halcyon.ini file (default off).

By default, this restores the previous behavior of synchronous CF writes that block the session.

appurist commented 6 years ago

This is not an option for anyone to use, ever, in the future, except for me during this WIP testing phase. It exists to avoid having to update code on a production grid only to enable testing of this code that already exists in the region server. Other than my specific tests, this will always be hard-coded off, or if we find some way to work around the side effects, it will be hard-coded on and the option will be removed. However, for now I'll add it to the sample file with a comment to never use it.

mdickson commented 6 years ago

If the INI option is developer only is there a good reason to actually include it? Why not set the change in code and test and decide how to run it before its merged to HEAD.

appurist commented 6 years ago

It exists to avoid having to update code on a production grid only to enable testing of this code that already exists in the region server.

This is a small no-brainer change that is part of enabling more real-world testing on developer test regions (as we have done many times) such as Zauber's and Mike's regions (and in most cases Sandboxes).

I'm trying to package up an update that can be tested and will provide all the other commits to the full grid while not eliminating this WIP from the same build. It will provide us with a disabled version for the Sandboxes and possible rollout while allowing parallel progress on developer test regions.

I'm not looking for feedback on whether this should be done, but rather whether there are any bugs/typos etc. (Although I have verified the change with breakpoints etc before committing.) The review request is for a sanity check on the actual code changes.