Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There's something strange going on with hierarchies on Works. #2500

Closed
paul-butcher opened this issue Dec 18, 2023 · 19 comments
Closed

There's something strange going on with hierarchies on Works. #2500

paul-butcher opened this issue Dec 18, 2023 · 19 comments
Assignees

Comments

@paul-butcher
Copy link
Contributor

https://wellcome.slack.com/archives/C8X9YKM5X/p1702638185461429

https://wellcomecollection.org/works/b62y2grn is erroneously appearing as a child of https://wellcomecollection.org/works/vsyg427x within the FPA archive.

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

My initial thought is that the candidate causes for this kind of thing include:

  • b62y2grn is the result of a merge between something that belongs in the hierarchy, and something that doesn't - possibly as a result of something odd in the source data or as a result of initial transformation.
  • Sierra hierarchies are built from a slight paucity of data. Whereas all CALM records know exactly where they are in a hierarchy, Sierra records only know about their parents (and children, but that relationship is currently ignored). It is simply enough that the record has children). Perhaps vsyg427x is being marked as the parent of b62y2grn due to some kind of identifier clash (or transformer/source data issue)

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

in /pipeline/matcher/scripts $ yarn getMatcherGraph b62y2grn returns a graph with a single node, so it should not be merging with anything.

Looking at works-identified-2023-11-09/_doc/b62y2grn it has no mergeCandidates

...
"state": {
      "sourceIdentifier": {
        "identifierType": {
          "id": "sierra-system-number"
        },
        "ontologyType": "Work",
        "value": "b33292656"
      },
      "canonicalId": "b62y2grn",
      "sourceModifiedTime": "2023-12-13T12:00:48Z",
      "mergeCandidates": [], <-------------HERE-----------
...

@paul-butcher
Copy link
Contributor Author

Also in works-identified-2023-11-09/_doc/b62y2grn, it has a collectionPath with no slashes, which means it expects to be the root of its own hierarchy.

...
    "collectionPath": {
        "path": "3329265i"
      },
  ...

This property is set from the Sierra MARC field 001 on any documents that contain fields of type 774, which can be seen here (VPN required)
b3329265

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

Oddly, in works-merged (works-merged-2023-11-09/_doc/b62y2grn), the document has a collectionPath of "SAFPA/CB/2/41/43", which is exactly where it is curently being displayed. I don't yet know what could have set collectionPath like this.

There are no redirectSources in the version in works-merged, so nothing has been merged into it.

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

Another possibility in cases like this is that there was a source data problem, and although it has since been resolved, the pipeline has hit a snag, and it's not been updated past a certain stage. (it doesn't look like this is the case here)

The final (indexed) version has the following times:
"modifiedTime": "2023-12-13T12:00:48Z"
"mergedTime": "2023-12-13T12:03:59.519348Z",
"indexedTime": "2023-12-13T12:23:39.438640Z",

The merged version has the following times in its state:
"mergedTime": "2023-12-13T12:03:59.519348Z",
"sourceModifiedTime": "2023-12-13T12:00:48Z",

The identified version
"sourceModifiedTime": "2023-12-13T12:00:48Z",

So at every stage, it has the same source modified time, which suggests that they are all built from the same version of the source data. Similarly, mergedTime matches between merged and indexed and the times on the later stages are after those on the earlier stages. So it looks like this document has passed through the system as expected.

@paul-butcher
Copy link
Contributor Author

Just to be sure, I've checked the DLQs. the merger has some (7) messages on its DLQ, five of which are sub3yrcq(which is part of a different story. However, the three distinct messages there do not look relevant (sub3yrcq , zzdb6wka and dd63gq8m).

So this points to a problem somewhere in the relation embedder (including the path concatenator)

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

The path concatenator is where that paucity of data mentioned in the first comment is resolved. When faced with a path branch/leaf it looks up root/branch and returns root/branch/leaf.

It's not obvious how this could result in what we are seeing here, but it's worth a look. This has highlighted to me that the path concatenator hampers observability

@paul-butcher
Copy link
Contributor Author

This is interesting. The James Gardiner Collection has suffered the same fate, and in the same position.

If you visit the preceding or following sibling, and refresh the page (the hierarchy is not reloaded when you just click on the link), then the Drawings of Childbirth entry in the tree changes to James Gardiner.

This makes it look like there's something special about the "parent" record that's making it grab hold of these two collections.

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

Using the message injector, I pushed b62y2grn onto id_minter_output.

If this does not fix the problem, then there is something awry in the processing of that document itself. If it does, then either this is something wrong with vsyg427x or it was some strange ephemeral thing I may not be able to reproduce.

I will eventually do the same with ctu3s9j3 and vsyg427x to get everything back to normal, but I want to check the behaviour one-at-a-time.

@paul-butcher
Copy link
Contributor Author

I wonder if something funny happened in the Batcher. See logs

All those 332963i/3329263i.nn paths are part of the Rigshospitalet tree, In this log in particular, they are all in an input batch with SAFPA/CB/2/41/43.

However, the selectors created look correct - it correctly identifies three distinct trees and provides the selectors for them.

@paul-butcher
Copy link
Contributor Author

b62y2grn has gone all the way through now, and the problem is still there.

@paul-butcher
Copy link
Contributor Author

So I tried vsyg427x, with no success. Though now the Rigshospital content is the one that appears in that slot. I assume that the last one to be processed gets presented there. James Gardiner is still stuck in the tree if you go straight to it.

@paul-butcher
Copy link
Contributor Author

Got it! This is an interesting one, so I thought I'd just exclaim in here, then explain in the next comment.

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 18, 2023

In the path concatenator, when a record is trying to find its parent or its children, it splits off the head or tail of its own path, then constructs a wildcard query for it.

So, given trunk/branch, it looks for its parent */trunk and its children branch/*

All paths are constructed with this / path separator.

When we have a path with just one segment (i.e. it is the root of its own tree), it still does the same attempt at a query, using the split path. However, the head of the path, as used by the query, is not the whole of the path(because it's looking for the parent of this record), but an empty string. So the parent query looks for */. The tail is the whole string, so root works as expected to find its children: root/*.

(admittedly this has always been inefficient, and it has now proven to be incorrect as well)

The / separator is expected to only go between path segments. We don't expect to find it at the start or end of a collectionPath.

Enter hzg8xkez (b1953498x), which has a collectionPath of SAFPA/CB/2/41/43/

So now, any document that is ingested as the root of its own tree is erroneously turned into a child of SAFPA/CB/2/41/43/. Even better, because it has happened due to the lack of its own path segment, it becomes SAFPA/CB/2/41/43/, which is then "corrected" to SAFPA/CB/2/41/43.

@paul-butcher
Copy link
Contributor Author

This will need some reindexing to fully fix, but if we're lucky, then it has only hit the ones mentioned above, so I can do a targeted reindex of just those.

I'll do some analysis today to see how widespread it is.

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 19, 2023

There are 48 records that have been affected by this, so it's simple enough to just push those through when we're ready.

works-merged-2023-11-09/_search  {
  "query": {
    "bool":{
    "filter": {
      "wildcard": {
        "data.collectionPath.path.keyword": {
          "value": "SAFPA/CB/2/41/43"
        }
      }
    } 
    }
  }  ,
  "_source":false,
"size": 100
}

@kenoir
Copy link
Contributor

kenoir commented Dec 19, 2023

There are 48 records that have been affected by this

How did you find those records?

@paul-butcher
Copy link
Contributor Author

paul-butcher commented Dec 19, 2023

By running the query in that comment on works-merged-2023-11-09 in pipeline-2023-11-09 (using the developer console in ES).

I found hzg8xkez earlier in the thread with a similar query containing "value": "*/" (in fact, this is why the 48 record query uses an unnecessary wildcard query, because I just used the same JSON and changed the query term), and discovered it is the only record with a trailing slash. So all the records that have been hit by this issue will have a path of SAFPA/CB/2/41/43.

Then I turned it into a list of ids for use in the message injector mentioned here
thus:

$ grep "_id" 'response.json' | sed 's/.*: "//' | sed 's/".*//'

I think I will also have to reload hzg8xkez, to get it back into its rightful position, and purge whichever cuckoo is sitting in its place under vsyg427x at the time. (so it's actually 49)

@paul-butcher
Copy link
Contributor Author

This is all fixed and working.

However, you may still see the wrong tree due to caching. I have checked with a different browser, and both https://wellcomecollection.org/works/b62y2grn and https://wellcomecollection.org/works/ctu3s9j3 look OK.

https://wellcomecollection.org/works/pss8y4yn shows that the James Gardiner Collection is back to normal and https://wellcomecollection.org/works/d6zfhy2b shows that the Rigshospitalet collection is back to normal, even if the cached version appears not to be.

@github-project-automation github-project-automation bot moved this from Ready for review to Done in Digital platform Dec 19, 2023
@pollecuttn pollecuttn moved this from Done to Archive in Digital platform Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants