Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006105opensim[REGION] OpenSim Corepublic2012-07-25 02:272014-07-29 13:41
Assigned Tojustincc 
PlatformOSOS Version
Product Versionmaster (dev code) 
Target Versionmaster (dev code)Fixed in Version 
Summary0006105: [PATCH] Support multi-region OAR files
DescriptionThis patch extends the OAR format to allow saving multiple regions in a single file.

The save-oar command still creates single-region OARs by default (OAR version 0.8). It saves the new format if the "--all" parameter is specified. The new format has version number 1.0 because older versions of OpenSim can't read it.
Additional InformationThe layout of the OAR file is now as follows. Each region is stored in a separate directory, but the assets are shared:


The regions' directory names include the region's location (relative to the root region) in order to ensure uniqueness even if regions have duplicate names.

The file archive.xml contains a manifest of the included regions. The list of regions always describes a rectangle, with the root region (the region where the "save-oar" command was run) in the SW corner. Missing regions are supported, and are represented by empty elements.

For example:

<?xml version="1.0" encoding="utf-16"?>
<archive major_version="1" minor_version="0">
TagsNo tags attached.
Git Revision or version number777cbc00585ecab724eb541c9cdd4f77e1448613
Run Mode Grid (Multiple Regions per Sim)
Physics EngineODE
Environment.NET / Windows64
Mono VersionNone
Attached Filespatch file icon 0001-Fixed-a-rare-bug-that-caused-Save-OAR-to-fail-becaus.patch [^] (3,454 bytes) 2012-07-25 02:27 [Show Content]
patch file icon 0002-When-loading-an-OAR-validate-the-Group-ID-s-and-the-.patch [^] (6,277 bytes) 2012-07-25 02:28 [Show Content]
patch file icon 0004-Renamed-ArchiveWriteRequestPreparation-to-ArchiveWri.patch [^] (59,619 bytes) 2012-07-25 02:28 [Show Content]
patch file icon 0003-Support-multi-region-OAR-files.patch [^] (101,713 bytes) 2012-08-27 02:51 [Show Content]
patch file icon 0001-Rename-ArchiveWriteRequestPreparatio-nto-ArchiveWrit.patch [^] (59,916 bytes) 2012-09-07 15:41 [Show Content]
patch file icon 0006-Added-unit-tests-for-multi-region-OARs.patch [^] (22,618 bytes) 2012-09-12 07:48 [Show Content]
patch file icon 0001-Fixed-saving-non-square-multi-region-OARs.patch [^] (1,179 bytes) 2012-10-24 06:10 [Show Content]

- Relationships

-  Notes
orenh (administrator)
2012-07-25 02:29

For a discussion of this feature see: [^]

Once this is approved I'll update the OAR documentation in the Wiki.
orenh (administrator)
2012-07-25 02:36

This patch makes [^] irrelevant.

This patch might also make [^] irrelevant (saving megaregion terrains), because a multi-region OAR already stores all the terrains. I'm not familiar with megaregions so the issue owner should check this. That issue seems to have been abandoned for now, but I added a note in that issue anyway.
justincc (administrator)
2012-08-02 17:27
edited on: 2012-08-02 17:34

Regarding the comment about not saving parcel owner IDs in patch 0002, this is actually an uncaught regression - deserialization was performed in 0.7.2. Do you forsee any problem with restoring this? Urgh, that's what happens when you're lazy in a regression test and don't actually check everything.

justincc (administrator)
2012-08-02 17:29

Why are you proposing to rename ArchiveWriteRequestPreparation to
 ArchiveWriteRequest? Is this because the 'Preparation' is doing some writing? I'm not sure how much is gained from renaming AWRP to AWR though, since AWRE is then slightly more confusing.
justincc (administrator)
2012-08-02 18:38

Hi Oren, I'm going to separate the response about 0003 into format and implementation.

  * You might have to deal with regions on the same simulator that have the same name. I can imagine this doesn't make much sense but there is currently an option to allow it (AllowDuplicateNames in GridService). Either this option needs to be squashed (I'm not sure if its even valid in terms of the rest of the system) or regions need to have their UUIDs attached to dir names in some manner.

  * Regions can contain slashes in their names, which need to be escaped. But to be fair, this is a general archive bug that needs to be fixed.

  * Could you consolidate information about format and any relevant implementation details at [1] as they're now spread out across this mantis and the mailing list posts?

  * Do you have an opinion on the memory impact of multi oar load? Loading large single OARs is already a problem - one that could probably be improved by loading data in a different order (e.g. not loading all objects before instantiation). This didn't use to be possible since older OAR formats did not save other data in the tar file in the right order.

  * Why do ResolveUserUuid by scene? I know module/service retrieval is shitty (i.e. why do we need to go through a scene to get these things?) but these will always be the same across all scenes. I would rather not increase complexity by implying that this is not the case within the code.

  * Ah, I see AWRE was combined in AWRP. What is your general thinking here? My initial thought is that I would much prefer to keep them separate across the async asset collection and writing boundary, in order to reduce complexity.

  * I would not want to include a change of this magnitude without regression tests that exercise the new multi-region functionality and changed aspects of the OAR format where applicable.

[1] [^]
orenh (administrator)
2012-08-03 00:02

* Not restoring parcel owner IDs: that doesn't cause problems; it just means the estate owner is always set as the owner of the parcels. Which I think should be the case anyway, since when a user loads an OAR they presumably expect to get full permissions over the resulting region.

* The new format does support multiple regions with the same name. That's why the subdirectories where the regions are stored contain each region's coordinates in them. E.g.: "1_1_MyRegion", "1_2_MyRegion", etc. Using coordinates for the directory names is better than using the UUID because it's more readable, and also provides good ad-hoc understanding of the OAR for someone who's looking at its contents.

* I think that the OAR format should be changed to a ZIP file, because ZIP files have a directory structure, which makes it easy to read the files in the order *we* choose instead of the order in which the files were archived. This would make loading more efficient since we would be able to handle each piece of data immediately after reading it. But that's out of scope for this patch.

* This is related to the reason I merged AWRE and AWRP. Efficiency requires handling data as soon as possible after reading it. Therefore, there aren't separate "read" and "write" phases: they are intertwined.

* I'll add documentation to the wiki, and unit tests.
orenh (administrator)
2012-08-03 00:22

Documented this feature proposal here: [^]
justincc (administrator)
2012-08-03 19:35

Ah sorry orenh - I accidentally interpreted your comments about parcel owner IDs as saying there would be no issue if they were restored. I have now added stripping of these to the --publish save oar option. However, it might also be an idea to have a "load oar" option that will ignore these even if they are present.
justincc (administrator)
2012-08-03 19:36

Could you update patch 0002 to take account of these changes? Then I can peer-check/apply this and take an in-depth look at 0003. However, just fyi I'm technically on holiday next week so my response times might be rather delayed.
orenh (administrator)
2012-08-03 23:48

Regarding "--publish": I think it would be better to remove this option, and instead add an option called "--ignore-owners" to Load OAR. That places the power to decide whether or not to respect ownership with the person who loads the OAR rather than the person who saves it. That is probably correct since anyone who can Load OAR can also change the permissions anyway. I actually have code to ignore owners (it's just a few lines), and I could submit a patch to do that.
justincc (administrator)
2012-08-20 14:03

I applied patch 0002 as git master 812c498 with adaptations.
justincc (administrator)
2012-08-20 14:11

I regard the --publish option as important for people wanting to make their OARs generally available. It's too much to expect receivers to have to specify --ignore-owners every time.

I think that if you 'receive' a copy of a digital good then you own that copy (creator information can still be preserved). Of course, there are lots of models where one only has a license to a digital good. But really here we're fighting against the current limitations of the SL system.

Having said that, I would have no problem with a --ignore-owners flag on "load oar" as well as --publish on "save oar".
justincc (administrator)
2012-08-20 15:02

A few more questions about the file format at [1].

[1] [^]

If we can answer the few remaining format and implementation questions above then I think I would be happy with it. However, I still need 0003 to be sorted out to apply against current OpenSim master with regression tests in order for me to fully assess the implementation details.

If possible and you're willing to do the extra work, a separate 'new format' patch could be applied before a separate patch for the implementation changes. However, I certainly understand if this is not possible or you don't want to go down this route.

Just to be clear, I also wouldn't regard there as being final 'agreement' on this until the code to implement this is in core OpenSimulator. And even then, it would be subject to change until it gets out in a release, though in practice I wouldn't anticipate any breaking change unless there's something is seriously overlooked that would be extremely difficult to change and maintain backward compatibility with multi-oars.
orenh (administrator)
2012-08-27 02:53

I made a minor change to patch 0003 to take into account the latest changes in Master, and added patch 0005 with unit tests for multi-region OARs.
justincc (administrator)
2012-09-07 15:41

0001-Rename-ArchiveWriteRequestPreparatio-nto-ArchiveWrit.patch is an alternative patch for 0004-Renamed-ArchiveWriteRequestPreparation-to-ArchiveWri.patch since that no longer applies
justincc (administrator)
2012-09-07 15:45

Oren, I was able to successfully apply 0003 and my adapted patch for 0004. However, on applying 0005 and rerunning the entire regression test suite, test halted with the following failures

     [exec] Errors and Failures:
     [exec] 1) Test Failure : OpenSim.Region.Framework.Scenes.Tests.ScenePresenceTeleportTests.TestSameSimulatorNeighbouringRegionsTeleport
     [exec] Expected: not null
     [exec] But was: null
     [exec] at OpenSim.Region.Framework.Scenes.Tests.ScenePresenceTeleportTests.TestSameSimulatorNeighbouringRegionsTeleport () [0x00000] in <filename unknown>:0
     [exec] 2) Test Failure : OpenSim.Region.Framework.Scenes.Tests.ScenePresenceTeleportTests.TestSameSimulatorSeparatedRegionsTeleport
     [exec] Expected: null
     [exec] But was: <OpenSim.Region.Framework.Scenes.ScenePresence>
     [exec] at OpenSim.Region.Framework.Scenes.Tests.ScenePresenceTeleportTests.TestSameSimulatorSeparatedRegionsTeleport () [0x00000] in <filename unknown>:0

I expect this is because you stopped creating a new SceneManager for each test in SceneHelpers (the VM is not restarted for each test).

I want to avoid new statics wherever possible such as the SceneManager.Instance. These tend to make regression tests more difficult/complicated and make constructions of tests for performance analysis, etc. much more complicated. And from a casual glance at 0005, I'm not sure it's even necessary.

Please could you make sure that 0005 passes ALL the regression tests please. Once this happens I will be close to committing this patch and sorting out any issues that come up post-patch (I assume you'll be happy to deal with any bugs that come up in a timely fashion).
justincc (administrator)
2012-09-07 15:47

Also, I would like your opinion on the remaining comments in, [^] especially how this will act if there are duplicate region names or whether we need to investigate this further.

In addition, how are you handling situations where the layout on the receiving simulator does not match that in the multi-oar? Are the mismatching regions ignored?
orenh (administrator)
2012-09-12 07:55

Sorry about the unit tests! They are now fixed: I deleted patch 0005, and added patch 0006 instead.

Regarding your questions: I answered them in [^] , but in short the answers are:

1. How this will act if there are duplicate region names?

The new format supports multiple regions with the same name, because each region's directory contains the region's relative position and that position is unique within the OAR.

2. How are you handling situations where the layout on the receiving simulator does not match that in the multi-oar?

a) If the receiving simulator contains a "hole" where one of the OAR's regions should go then that region isn't loaded.

b) If the receiving simulator contains extra regions (beyond the area that the OAR defines) then they are unchanged.
justincc (administrator)
2012-09-14 15:03

0003-Support-multi-region-OAR-files.patch was committed as ce46821.
0001-Rename-ArchiveWriteRequestPreparatio-nto-ArchiveWrit.patch was committed as d7e6fe4.
0006-Added-unit-tests-for-multi-region-OARs.patch was committed as 5dd2569.

Thanks Oren. Could you keep an eye on Mantis for any issues that pop up in the coming weeks?

Regarding the layout, I would have thought that it would be good to warn the user (probably with a y/n confirmation) if they were trying to load a multi-oar into a region set with the wrong layout. At a level beyond that, one could perhaps generate the missing regions automatically if the user agrees.
justincc (administrator)
2012-09-18 17:08

Also, as this is now in, please could you add the documentation in the wiki. Thanks.
orenh (administrator)
2012-09-22 23:44

Updated the Wiki: [^] [^]
Gwyneth Llewelyn (reporter)
2012-10-22 12:47
edited on: 2012-10-22 12:54

Under which circumstances would archive.xml just list the first region when --all is selected, even though the remaining regions are all there?

I've currently got an instance running 3 sims, side-by-side. When saving them all, what I get is regions/* perfectly correct according to the specs (e.g. subdirectories correctly named 1_1_RegionOnTheSWCorner, 2_1_RegionInTheMiddle, and 3_1_RegionOnTheSECorner). All assets are there as expected, too. But assets.xml just shows information for 1_1_RegionOnTheSWCorner. The remaining two regions are missing.

Is it because my original instance just has a 1x3 map instead of a 3x3 map? According to the wiki page for the OAR 1.0 specs, this should be irrelevant — missing regions should just be, well, missing (and ignored when uploading). In fact, the assets.xml file adds the two missing rows as expected (i.e. just a <row /> line on the file). But it's also "missing" the remaining two regions. It almost looks like that the algorithm confused the 1x3 map with a 3x1 map while writing assets.xml.

I'm not sure if this is an issue worth reporting or just missing information somewhere. E.g. "make sure you start with a square map or this won't work". Maybe I'm just doing a systematic error somewhere.

orenh (administrator)
2012-10-23 01:48

You're right: in non-square maps the width and height are reversed. I'll submit a patch for that soon.
Gwyneth Llewelyn (reporter)
2012-10-23 03:40

Thanks, orenh!
orenh (administrator)
2012-10-24 06:11

Added patch: 0001-Fixed-saving-non-square-multi-region-OARs.patch

This fixes a bug that occurred when saving a non-square multi-region OAR.
justincc (administrator)
2012-10-25 16:13

orenh, happy to apply the patch but please could you put what is being fixed (correct revered width and height of non square maps) in the commit message. Thanks.
justincc (administrator)
2012-10-25 16:16

Alright, turns out I accidentally applied it anyway.

- Issue History
Date Modified Username Field Change
2012-07-25 02:27 orenh New Issue
2012-07-25 02:27 orenh Status new => assigned
2012-07-25 02:27 orenh Assigned To => justincc
2012-07-25 02:27 orenh File Added: 0001-Fixed-a-rare-bug-that-caused-Save-OAR-to-fail-becaus.patch
2012-07-25 02:28 orenh File Added: 0002-When-loading-an-OAR-validate-the-Group-ID-s-and-the-.patch
2012-07-25 02:28 orenh File Added: 0003-Support-multi-region-OAR-files.patch
2012-07-25 02:28 orenh File Added: 0004-Renamed-ArchiveWriteRequestPreparation-to-ArchiveWri.patch
2012-07-25 02:29 orenh Note Added: 0021891
2012-07-25 02:36 orenh Note Added: 0021893
2012-07-25 02:39 orenh Status assigned => patch included
2012-08-02 17:27 justincc Note Added: 0021967
2012-08-02 17:29 justincc Note Added: 0021968
2012-08-02 17:34 justincc Note Edited: 0021967 View Revisions
2012-08-02 18:38 justincc Note Added: 0021970
2012-08-02 18:38 justincc Status patch included => patch feedback
2012-08-03 00:02 orenh Note Added: 0021972
2012-08-03 00:22 orenh Note Added: 0021973
2012-08-03 19:35 justincc Note Added: 0021977
2012-08-03 19:36 justincc Note Added: 0021978
2012-08-03 23:48 orenh Note Added: 0021979
2012-08-20 14:03 justincc Note Added: 0022396
2012-08-20 14:11 justincc Note Added: 0022397
2012-08-20 15:02 justincc Note Added: 0022399
2012-08-27 02:47 orenh File Deleted: 0003-Support-multi-region-OAR-files.patch
2012-08-27 02:51 orenh File Added: 0003-Support-multi-region-OAR-files.patch
2012-08-27 02:52 orenh File Added: 0005-Added-unit-tests-for-multi-region-OARs.patch
2012-08-27 02:53 orenh Note Added: 0022461
2012-08-27 02:53 orenh Status patch feedback => patch included
2012-09-07 15:41 justincc File Added: 0001-Rename-ArchiveWriteRequestPreparatio-nto-ArchiveWrit.patch
2012-09-07 15:41 justincc Note Added: 0022531
2012-09-07 15:45 justincc Note Added: 0022532
2012-09-07 15:45 justincc Status patch included => patch feedback
2012-09-07 15:47 justincc Note Added: 0022533
2012-09-12 07:47 orenh File Deleted: 0005-Added-unit-tests-for-multi-region-OARs.patch
2012-09-12 07:48 orenh File Added: 0006-Added-unit-tests-for-multi-region-OARs.patch
2012-09-12 07:53 orenh Note View State: 0022533: private
2012-09-12 07:53 orenh Note View State: 0022533: public
2012-09-12 07:55 orenh Note Added: 0022607
2012-09-14 15:03 justincc Note Added: 0022610
2012-09-18 17:08 justincc Note Added: 0022633
2012-09-22 23:44 orenh Note Added: 0022667
2012-10-22 12:47 Gwyneth Llewelyn Note Added: 0022887
2012-10-22 12:54 Gwyneth Llewelyn Note Edited: 0022887 View Revisions
2012-10-23 01:48 orenh Note Added: 0022895
2012-10-23 03:40 Gwyneth Llewelyn Note Added: 0022898
2012-10-24 06:10 orenh File Added: 0001-Fixed-saving-non-square-multi-region-OARs.patch
2012-10-24 06:11 orenh Note Added: 0022906
2012-10-24 06:11 orenh Status patch feedback => patch included
2012-10-25 16:13 justincc Note Added: 0022927
2012-10-25 16:16 justincc Note Added: 0022928
2012-10-25 16:16 justincc Status patch included => resolved
2012-10-25 16:16 justincc Resolution open => fixed
2014-07-29 13:41 chi11ken Status resolved => closed

Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker