search

marklogic-community / roxy

Deployment tool for MarkLogic applications. Also provides optional unit test and XQuery MVC structure
Other
87 stars 66 forks source link

Fixing Issue #850: bootstrap broken in ML 8.0-7 #852

Closed joecrean closed 6 years ago

joecrean commented 7 years ago

Since the upgrade to MarkLogic 8.0-7 roxy bootstrap fails in an environment where the application was already deployed (and by converse it does not fail in a fresh environment).

Path namespaces are removed and then added every time bootstrap runs. In order to remove them we first need to check if they are being used. They can be used in Path Range Index definitions, elements containing Geospatial co-ordinates and Field definitions. The API used to delete the path namespaces is called admin:database-delete-path-namespace. This API will check to see if the namespaces are in use before deletion and in 8.0-7 this has been tightened up to additionally check field definitions.

Up to now roxy has simply deleted and re-added ALL path range indexes and their namespaces which in itself could cause lots of re-indexing. Now with the stricter API we would need to delete and re-add field range indexes. This is clearly a bad direction and for that reason this solution, detects what has changed and only removes indexes when it is really necessary. #850

grtjn commented 7 years ago

Thnx! Looks good at quick glance..

RobertSzkutak commented 6 years ago

A major customer is being impacted by this. Will perform a review...

RobertSzkutak commented 6 years ago

Overall this looks fine to me. Once concern I have is that this doesnt address these new geospatial indexes in ML9 : https://docs.marklogic.com/admin:database-get-geospatial-region-path-indexes

So presumably this would drop paths being used exclusively by these indexes which I imagine could cause issues. On the other hand, we don't support creating these indexes in Roxy yet either so...

joecrean commented 6 years ago

Hi @RobertSzkutak sorry it took a while to get back to you on this. You are right I missed out on the above - that is because I am working with ML 8 so i didn't have any visibility on that new API. In any case if we enhance to cover it we will have to ensure the we also test for version 9 as well. I can build this in if you want..

grtjn commented 6 years ago

Related to #863 too..

grtjn commented 6 years ago

I think the general idea is good. Rather than delete-all, create-all on path-namespaces, it is better to only remove unneeded, and add new ones instead.

I am less fond of deleting indexes as part of apply-path-namespaces though. I think it would be better to delete unneeded indexes first, then only delete/create namespaces as needed, and add new indexes. That would be a cleaner approach i think.

Good to see the abundant use of maps, iterator operator, and function invocation, but not sure it makes the code easier to read, particularly since most other code uses old-fashion style xqy.

I'll try to open an alternative PR later this week, and see if I can include the changes of #864 in that as well..

grtjn commented 6 years ago

People have been waiting a while for this, sorry for the long delay! There were some PR's from @tdiepenbrock that interfered with this, and actually solved the same issue, while adding enhancements at the same time.

I replaced this PR with PR #866, which should effectively do the same, and adds support for metadata fields and the relatively new geo path indexes. Thanks very much @joecrean. I ended up choosing Tom's code over yours, but it was definitely useful nonetheless. It helped straighten thoughts, and focus on the issue. I couldn't have produced my PR without you and Tom's help this quickly..