Performance bottleneck in nested model conversion for Fedora persister

[dolsysmith]

Cross-referencing this issue on the Samvera/Hyrax repo.

When using the Fedora persister with Hyrax 5/Fedora 6, I'm seeing a bottleneck in saving ACL objects (causing performance to degrade as the size of the repository scales). I believe that the root cause lies in the behavior of the nested_graph method of Fedora::Persister::ModelConverter::NestedProperty.

Because Hyrax::AccessControl objects nest Hyrax::Permission objects within themselves, saving the ACL triggers the creation of a nested graph. However, the instance of ModelConverter created when populating the nested graph receives a value for subject_uri that causes the graph to be populated with all of the objects under the top-level container in the repository (which in turns causes performance degradation).

Here is the code I'm referring to:

@nested_graph ||= ModelConverter.new(resource: Valkyrie::Types::Anything[value.value], adapter: value.adapter, subject_uri: subject_uri).convert.graph

And subject_uri comes from this method:

   def subject_uri
          @subject_uri ||= ::RDF::URI(RDF::Node.new.to_s.gsub("_:", "#"))
   end

As a result, the subject_uri passed to the ModelConverter instance for the Hyrax::Permission resource has a value like this: #<RDF::URI:0x927e8 URI:#g600020>. And this truncated URI is then passed to the Fedora API, which returns all objects directly under the base path (e.g., http://localhost:8080/fcrepo/rest/development).

This behavior, which occurs only for nested properties, differs from the non-nested case, where instances of ModelConverter are created with an empty RDF::URI.

To demonstrate, I inserted some logging into the initializer for the ModelCoverter class. Here are the results of the calls to this initializer when persisting an update to a single ACL:

[{'model': '#<GwEtd',
  'id': '"8aad0784-3e7a-4a9a-9348-c72fb3a0b80e">',
  'subject_uri': '#<RDF::URI:0x92608 URI:>'},
 {'model': '#<Hyrax::AccessControl',
  'id': '"7a20d68e-dd96-4a56-8215-9fb7b7a6f5bc">',
  'subject_uri': '#<RDF::URI:0x926f8 URI:>'},
 {'model': '#<Hyrax::Permission',
  'subject_uri': '#<RDF::URI:0x92748 URI:#g599860>'},
 {'model': '#<Hyrax::Permission',
  'subject_uri': '#<RDF::URI:0x92798 URI:#g599940>'},
 {'model': '#<Hyrax::Permission',
  'subject_uri': '#<RDF::URI:0x927e8 URI:#g600020>'}]

(The model and id values have been extracted from the resource argument to the initializer.)

Can the model converter for the nested property be initialized with an empty URI, just like the other resources? Is there a need for the anonymous node when working with nested resources?

[tpendragon]

@dolsysmith Yay! I'm so glad to have this adapter under production use. It's very likely there's optimizations to be had here - I think it'd be great if you tried it and see if it made a difference.

I assume what I was trying to make happen was create blank nodes within the parent graph, so that when you request that node any nested resources come back with it (like would happen in postgres, solr, or any other serialization - if you wanted real resources with real URIs, I think that's something else probably?) If that's not supported by Fedora, or something else needs to happen to make that work, then we should change it. Maybe the URIs for those should be calculated differently, I'm not sure.

Questions we'd have to answer for production release:

1. Would changing that behavior break reading or writing objects that had been made before?
   • Is there a compatible way we can implement?
2. Should we release this as a major version?

[dolsysmith]

@tpendragon Thank you for responding so promptly! Unfortunately, the simple fix I proposed doesn't appear to work -- it prevents permissions from getting created correctly by Hyrax. I'll keep digging...I did observe that (with the current behavior) when the permissions objects are actually created in Fedora, they have id values with slightly different URI's from the URI's that point to these objects from the parent object. That might be a red herring, but it struck me as odd.

[dolsysmith]

@tpendragon In my quest to better understand the behavior of persister and model converter, I'm rather stuck on this line in the ModelConverter.convert method:

graph_resource.graph.delete([nil, nil, nil])

What does the call to delete do?

[tpendragon]

What does the call to delete do?

@dolsysmith I assume this empties out the internal graph so that we can rebuild it from scratch.

[dolsysmith]

@tpendragon Would you mind elaborating on what you said here?

I assume what I was trying to make happen was create blank nodes within the parent graph, so that when you request that node any nested resources come back with it (like would happen in postgres, solr, or any other serialization - if you wanted real resources with real URIs, I think that's something else probably?)

This documentation on blank nodes in Fedora advises against their use; the hash URI's mentioned there are, I believe, what Hyrax uses to nest permission objects under the access control object that they describe (as in the screenshots above).

As far as I can tell, the issue in question occurs when ModelConverter.convert calls graph_resource on a Hyrax::Permission object that has already been saved (in the course of iterating over the properties of the parent ACL). At that point, the permission object already has an id property, but it's been set to just the hashed portion of the URI, not the full URI of the parent. I'm wondering if a solution might be as follows: a) changing the permission object's identifier to the full URI of the parent, plus the hashed portion, and b) using the parent identifier to initialize the graph returned by graph_resource.

But I don't know how that jibes with the original intention of these Valkyrie methods.

[tpendragon]

I assume what I was trying to make happen was create blank nodes within the parent graph, so that when you request that node any nested resources come back with it (like would happen in postgres, solr, or any other serialization - if you wanted real resources with real URIs, I think that's something else probably?)

Ah, sorry, it's been a while - yes, a skolemized blank node is what's intended.

Your proposed solution seems likely, I suggest trying it out and seeing if Fedora behaves the way you intend? The shared specs should be enough to validate that everything's working.

[dolsysmith]

Here's the fix I applied to our application, and it seems to work -- saving new works now takes 1-2 seconds on average, for a moderately sized repository, as opposed to several minutes.

I tested this patch against the Valkyrie spec suite, and everything passes. Let me know if you'd like me to submit a pull request.

[tpendragon]

@dolsysmith Amazing, that must be so satisfying! Sounds good to me - you should send a PR. I wonder if @dlpierce would be interested in testing it in Hyrax to make sure everything's playing nice?