IHTSDO / snowstorm

Scalable SNOMED CT Terminology Server using Elasticsearch
Other
204 stars 80 forks source link

Error creating code system version on child branch #383

Open Tannjorn opened 2 years ago

Tannjorn commented 2 years ago

Hi,

I get the following response when I am trying to create a code system version:

{ "error": "BAD_REQUEST", "message": "Recursion limit reached calculating MDRS in MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT for module 51000202101" }

The code system is VSOPOST-AT on branch as shown. SLVMAPS-AT has no code system, and therefore VSOPOST-AT has no dependant version.

Anyone knows what this means? (I have created versions on child branches like this before - without issues).

Kind regards, Jørn

kaicode commented 2 years ago

This is related to the Module Dependency Reference Set. When a code system is versioned it should have rows in the MDRS that describe the dependant modules. A feature was added to Snowstorm fairly recently to create these rows automatically. I expect the feature expects each ancestor branch of a code system to be a code system. It looks like the scenario you are attempting, where the code system parent branch has no code system, may no longer supported.

It looks like there is some configuration for this feature https://github.com/IHTSDO/snowstorm/blob/7.6.0/src/main/resources/application.properties#L771 . You could try adding the module you are versioning to the blocklist.. although that is just a guess on my part.. @pgwilliams are you able to provide any more information on this one please?

pgwilliams commented 2 years ago

Hi @Tannjorn yes the code is trying to calculate your Module Dependency Reference Set and is getting into a pickle because it can't work out the hierarchy between modules. We've had problems with this here and I've recently modified the code to prevent it bombing out the whole process (because we can probably live without an MDRS in the short term). See https://github.com/IHTSDO/snowstorm/commit/1d52caf3720bd1c28bb6696e964191c51c08fa1e

There was also a specific fix done for NO which confounded my ability to work out a hierarchy by putting most of its concepts in a submodule (which made it look dominant, but I was just wrong to take this approach) https://github.com/IHTSDO/snowstorm/commit/adc6324ac4c40723e3a869a259db1feea9e2a51e that's available in 7.6.0 - what version are you using?

There is also potentially a way to fix this by adding in an MDRS member so the hierarchy between modules is clearer, but I'm not sure if that will work in this particular case. Have you defined any additional modules in these branches?

Tannjorn commented 2 years ago

Thank's, I'll take a look on this and see where I can go.

I am running on 7.6.0, and there are no other modules on this branch. The branch only contains simple maps.

Tannjorn commented 2 years ago

@pgwilliams , could setting modules on the branch be a possible approach?

Tannjorn commented 2 years ago

@pgwilliams , refrasing my last question. Would updating the Module Dependency Refset in any way solve the problem? (This is running on a test-server, so I can do some testing).

pgwilliams commented 2 years ago

Hi @Tannjorn it might do yes. But I see that a fix I did is now available in version 7.7 so that's more likely to yield a positive result - see https://github.com/IHTSDO/snowstorm/commit/1d52caf3720bd1c28bb6696e964191c51c08fa1e

hurtigcodes commented 2 years ago

Hi @pgwilliams, I ran the same endpoint and branch as Jörn Andre did with 7.7.0 and got the result below.

I ran it with and without the config Kai suggested as well. I also cloned this branch and added the modules dependencies on the branch but this is difficult terrain. Even though I copied most of it from authoring I wonder if I set the effectiveTimes correctly and so on.

2022-03-22 13:30:35.894  INFO 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : Generating MDRS for MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT, international modules [900000000000012004, 57091000202101, 57101000202106, 51000202101, 449080006, 900000000000207008]
2022-03-22 13:30:35.894  WARN 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : CHECK LOGIC: 57091000202101 thought to be both International and an Extension Module
2022-03-22 13:30:35.895  WARN 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : Recursion limit reached calculating MDRS in MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT for module 51000202101. Falling back to assume dependency on Core Module
2022-03-22 13:30:35.895  WARN 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : CHECK LOGIC: 57101000202106 thought to be both International and an Extension Module
2022-03-22 13:30:35.896  WARN 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : Recursion limit reached calculating MDRS in MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT for module 51000202101. Falling back to assume dependency on Core Module
2022-03-22 13:30:35.896  WARN 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : CHECK LOGIC: 51000202101 thought to be both International and an Extension Module
2022-03-22 13:30:35.896  WARN 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : Recursion limit reached calculating MDRS in MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT for module 51000202101. Falling back to assume dependency on Core Module
2022-03-22 13:30:35.897  INFO 244763 --- [http-nio-8080-exec-3] o.s.s.c.d.s.ModuleDependencyService      : MDRS generation for MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT, modules [900000000000012004, 57091000202101, 57101000202106, 51000202101, 449080006, 900000000000207008] took 1.338443888s
2022-03-22 13:30:35.897  INFO 244763 --- [http-nio-8080-exec-3] io.kaicode.elasticvc.api.BranchService   : Rolling back commit on MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT started at 1647955834558
2022-03-22 13:30:35.897  INFO 244763 --- [http-nio-8080-exec-3] io.kaicode.elasticvc.api.BranchService   : Deleting documents on MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT started at 1647955834558.
2022-03-22 13:30:35.898  INFO 244763 --- [http-nio-8080-exec-3] io.kaicode.elasticvc.api.BranchService   : Clearing end time for documents on [MAIN/SNOMEDCT-NO/SLVMAPS-AT/VSOPOST-AT] ended at 1647955834558.
2022-03-22 13:30:35.909  WARN 244763 --- [http-nio-8080-exec-3] org.elasticsearch.client.RestClient      : request [POST http://localhost:9200/branch/_search?typed_keys=true&max_concurrent_shard_requests=5&ignore_unavailable=false&expand_wildcards=open&allow_no_indices=true&ignore_throttled=true&search_type=dfs_query_then_fetch&batched_reduce_size=512&ccs_minimize_roundtrips=true] returned 1 warnings: [299 Elasticsearch-7.17.0-bee86328705acaa9a6daede7140defd4d9ec56bd "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. Consider cold or frozen tiers in place of frozen indices."]
2022-03-22 13:30:35.931  WARN 244763 --- [http-nio-8080-exec-3] org.elasticsearch.client.RestClient      : request [POST http://localhost:9200/branch/_refresh?ignore_throttled=false&ignore_unavailable=false&expand_wildcards=open&allow_no_indices=true] returned 1 warnings: [299 Elasticsearch-7.17.0-bee86328705acaa9a6daede7140defd4d9ec56bd "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. Consider cold or frozen tiers in place of frozen indices."]
2022-03-22 13:30:35.932 ERROR 244763 --- [http-nio-8080-exec-3] o.s.s.rest.config.RestControllerAdvice   : null

java.lang.NullPointerException: null
    at org.snomed.snowstorm.core.data.services.ModuleDependencyService.lambda$generateModuleDependencies$2(ModuleDependencyService.java:236)
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:176)
    at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1621)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
    at org.snomed.snowstorm.core.data.services.ModuleDependencyService.generateModuleDependencies(ModuleDependencyService.java:237)
    at org.snomed.snowstorm.core.data.services.ReleaseService.createVersion(ReleaseService.java:55)
    at org.snomed.snowstorm.core.data.services.CodeSystemService.createVersion(CodeSystemService.java:234)
    at org.snomed.snowstorm.core.data.services.CodeSystemService$$FastClassBySpringCGLIB$$7d61f252.invoke(<generated>)
    at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218)
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:771)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
    at org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor.invoke(MethodSecurityInterceptor.java:69)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:691)
    at org.snomed.snowstorm.core.data.services.CodeSystemService$$EnhancerBySpringCGLIB$$70e9c71a.createVersion(<generated>)
    at org.snomed.snowstorm.rest.CodeSystemController.createVersion(CodeSystemController.java:144)
    at org.snomed.snowstorm.rest.CodeSystemController$$FastClassBySpringCGLIB$$51948cbe.invoke(<generated>)
    at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:687)
    at org.snomed.snowstorm.rest.CodeSystemController$$EnhancerBySpringCGLIB$$7e0de44.createVersion(<generated>)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:190)
    at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138)
    at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:105)
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:878)
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:792)
    at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
    at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:909)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:652)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:733)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at io.kaicode.rest.util.branchpathrewrite.BranchPathUriRewriteFilter.doFilter(BranchPathUriRewriteFilter.java:50)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.snomed.snowstorm.rest.security.RequestHeaderAuthenticationDecoratorWithRequiredRole.doFilterInternal(RequestHeaderAuthenticationDecoratorWithRequiredRole.java:43)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:103)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:320)
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:118)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:137)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:111)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:158)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:63)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:116)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:92)
    at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:77)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:105)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:56)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334)
    at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:215)
    at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:178)
    at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:358)
    at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:271)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:93)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:202)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:541)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:373)
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:868)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1589)
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    at java.base/java.lang.Thread.run(Thread.java:829)
pgwilliams commented 2 years ago

@Tannjorn @hurtigcodes First of all, I'm very sorry that you're having these issues which are obviously blocking you badly. I wrote this code which was intended to automate and simplify the generation of the MDRS (which as you say is complicated to work with) by determining how modules have been arranged. But there's such variation in how various countries have packaged their content that the code can get confused. I think what I need to do now (and should have done in the first place) is add in some configuration so that we can turn off this functionality if it goes wrong. So I'll raise a ticket for that internally now (INFRA-8655) to progress this.

Can I double check your structure with you here. It's one we've not come across before which is why the code is getting so confused and can't even work out if this is International or Extension! You've got a code systems defined below the SNOMEDCT-NO code system. But it uses the same module as SNOMEDCT-NO ? In what way is it a CodeSystem in this case? Could you maybe just lay this out for me a little, showing what is being added at each level? Are packages produced which the sub levels are then dependent on? We have a problem here with multiple levels of dependency and I think Snowstorm will have to evolve to make "dependencyPackage" an array rather than a single value.

Something that's probably very relevant is the fact that you say there's no dependent version. But you are dependent on NO. I think this is why the code can't tell if it's International or not, because of the lack of a dependent package. If I show you our configuration for Norway here, could you check it against yours?

See https://gist.github.com/pgwilliams/91188a61a0ed96c0ac70c754180889b2 in particular, could you check and ensure that you have a "previousDependencyPackage" defined on your SNOMEDCT-NO CodeSystem because it's the presence of THAT item that tell the MDRS code that it's working with an Extension. Solving that "is it International or is it an Extension" is definitely required, although may not be the whole story. Fingers crossed.

hurtigcodes commented 2 years ago

Hi @pgwilliams! No need for apologies, this issue seems to be our fault and obviously we're learning as we go along. I don't have all the details but I'll try to answer you to my best ability.

I talked to @Tannjorn and it seems he was able to solve this problem by essentially doing what @kaicode hinted at, namely creating a proper codesystem hierarchy with dependencies to each parent. He created a new codesystem on the parent, so that there are now three codesystems in total (four including MAIN).

We use the child (leaf node) to create and version content to a Norwegian medicines module, which is set up in the MDRS refset with a dependency to the Norwegian module as its parent. This approach is somewhat similar to how the Authoring project NO9 was set up, with the exception that we don't version content there yet so far as I know.

Lastly, I could find the property previousDependencyPackage on MAIN/SNOMEDCT-NO: "previousDependencyPackage": "xSnomedCT_InternationalRF2_MEMBER_20220131T120000Z.zip". (A side note: previousDependencyPackage value is the same as dependencyPackage, likely because we've injested the package twice, second time as a release patch to the dates rollup problem.)

pgwilliams commented 2 years ago

Hi @hurtigcodes @Tannjorn well if you were able to resolve the issue then I'll close this ticket, but I will progress code to allow this functionality to be bypassed if it were to again become problematic. In the past few days we've come across a situation that appears to be mutually (rather than hierarchically) dependent modules and again the code was not able to cope with that, although in that case it did not fail as catastrophically.

hurtigcodes commented 2 years ago

Hi @pgwilliams, request to re-open this ticket again. I'm trying to import the latest NO release onto this server and a similar type of error seems to occur:

`2022-04-19 13:37:07.668 ERROR 1428 --- [pool-2-thread-1] o.s.s.core.rf2.rf2import.ImportService : Failed RF2 SNAPSHOT import on branch MAIN/SNOMEDCT-NO. ID 0d36c5cb-319d-4bc9-a7ad-a61fb50fc85d

java.lang.NullPointerException: null at org.snomed.snowstorm.core.data.services.ModuleDependencyService.lambda$generateModuleDependencies$2(ModuleDependencyService.java:236) at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:176) at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1621) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578) at org.snomed.snowstorm.core.data.services.ModuleDependencyService.generateModuleDependencies(ModuleDependencyService.java:237) at org.snomed.snowstorm.core.data.services.ReleaseService.createVersion(ReleaseService.java:55) at org.snomed.snowstorm.core.data.services.CodeSystemService.createVersion(CodeSystemService.java:234) at org.snomed.snowstorm.core.data.services.CodeSystemService.createVersionIfCodeSystemFoundOnPath(CodeSystemService.java:282) at org.snomed.snowstorm.core.data.services.CodeSystemService$$FastClassBySpringCGLIB$$7d61f252.invoke() at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218) at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:687) at org.snomed.snowstorm.core.data.services.CodeSystemService$$EnhancerBySpringCGLIB$$3b1cf080.createVersionIfCodeSystemFoundOnPath() at org.snomed.snowstorm.core.rf2.rf2import.ImportService.importArchive(ImportService.java:115) at org.snomed.snowstorm.core.rf2.rf2import.ImportService.lambda$importArchiveAsync$1(ImportService.java:261) at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829)

`

pgwilliams commented 2 years ago

For sure lets re-open this issue. We should never be throwing a null pointer. Checking that line in the code I can see that it's right at the end of the function where the code only wants to persist records that have changed. So we're being passed a null effective time there. I should be able to re-produce this. Can you send me what JSON you're using to kick off your import process please, and I assume I can use the same most recent NO package.

I think I will add some config to allow this functionality to be skipped - it's obviously just too fragile and has bitten a number of users.

hurtigcodes commented 2 years ago

Thanks! Of course, here are the import params:

{
  "branchPath": "MAIN/SNOMEDCT-NO",
  "createCodeSystemVersion": true,
  "type": "SNAPSHOT"
}
pgwilliams commented 2 years ago

Super, thanks, I'll try that here.

Tannjorn commented 2 years ago

This is just a bit of information possibly explaining why this issue occured.

The use-case we tested was versioning maps on a sub-branch of a branch with Dailybuild content - including unversioned content in the map. An external application is to read a set of five stable maps, retrieving Snomed concepts for data on drugs. Then using this information to write an ECL and search for a Clinical Drug. And finally posting the relationship between the brand drug and Clinical drug back to Snowstorm. This map should be versioned seperately as is has a way more frequent cycle then needed for the terminology.

This was the set-up:

MAIN/SNOMEDCT-NO contained the NO-DailyBuild. MAIN/SNOMEDCT-NO/SLVMAPS contained basic stable maps. MAIN/SNOMEDCT-NO/SLVMAPS/VSOPOST was set to recieve maps from the tool.

MAIN/SNOMEDCT-NO was versioned with DailyBuild active. A CodeSystem on SLVMAPS could not exist without a dependencyRelease on SNOMEDCT-NO, so to read DailyBuild content from SNOMEDCT-NO, SLVMAPS was created on a branch without a CodeSystem. Because, creating a CodeSystem on a child-branch of a branch without a CodeSystem can (or actually must) be done without a dependant version - as no versions exist on the parent-branch.

We ended up with:

MAIN/SNOMEDCT-NO: SNOMEDCT-NO CodeSystem - version 20211015, DailyBuild MAIN/SNOMEDCT-NO/SLVMAPS; No CodeSystem MAIN/SNOMEDCT-NO/SLVMAPS/VSOPOST: VSOPOST CodeSystem.

Versioning VSOPOST worked well until the upgrade. After that we got the MLDS-error.

There is of course a great risk of integrity errors versioning maps with content from a DailyBuild, but there is a relevant use-case when developing maps that need way more frequent versioning than the release cycle for the terminology.

I am not sure if this set-up created the errors, or if it merely was affected by the underlying error. I neither can tell whether this, even after cleaning it up, created the problem with upgrading the server.

@pgwilliams, working further towards a production set-up, is it possible to version a map-branch using concepts that are NOT released in the underlying dependant version as referenced components in the map? Or with this trigger an integrity error when creating a version?

pgwilliams commented 2 years ago

@hurtigcodes I'm happy to say (for my sake, although not yours) that a Snapshot import onto our NO CodeSystem versioned cleanly. So that suggests we have some difference in our branch configuration that could be brought into line without needing a code fix. Here are my logs: https://gist.github.com/pgwilliams/03e9537c3ab0ce63f5f7b2eff78c61dc could you please send me the equivalent section in your logs when they fail please, from "Versioning content..." until the exception is thrown?

In the meantime I will also progress coding the option for a configuration switch to turn off this functionality.

@Tannjorn most of our issues with reference sets get picked up via external validation (eg our RVF process) so I suspect you might be OK but I'll flag @kaicode here for a 2nd opinion.