You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@any23.apache.org by lewismc <gi...@git.apache.org> on 2018/01/03 00:13:31 UTC

[GitHub] any23 pull request #56: ANY23-321 Add openie toggle functionality to service

GitHub user lewismc opened a pull request:

    https://github.com/apache/any23/pull/56

    ANY23-321 Add openie toggle functionality to service

    This issue is a large step towards addressing https://issues.apache.org/jira/browse/ANY23-321
    I am however not able to currently register the OpenIEExtractor within the ExtractorGroup/Factory when undertaking an extractor. I've debugged this down to the code in the URLClassLoader.addURL of [Any23PluginManager](https://github.com/apache/any23/blob/master/api/src/main/java/org/apache/any23/plugin/Any23PluginManager.java#L453). The OpenIE JAR's are dynamically loaded however the Extractor implementation does not seem to be registered when the extraction is executed.
    
    @ansell if you are able to pull this code and debug it would be greatly appreciated. I have specific lines for debugging if this would be helpful. Thank you in advance for any assistance here.
    
    P.S. you will also see that I've been making attempts to update the [plugin documentation](http://any23.apache.org/any23-plugins.html). 
    
    P.P.S I actually remember encountering a similar issue previously when attempting to register a plugin via the command line... I think dynamic ClassLoading is broken in Any23 right now. I am keen to fix it so any help here is appreciated folks.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lewismc/any23 ANY23-321

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/any23/pull/56.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #56
    
----
commit 706e891cf582736f90cfbe83bc1ef5d629e6dfd7
Author: Lewis John McGibbney <le...@...>
Date:   2018-01-03T00:05:39Z

    ANY23-321 Add openie toggle functionality to service

----


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @jgrzebyta Yes, I had that problem too. Mvn build fails, but that's not related to the classloader problem @lewismc is having (which my comment above should fix--the extractor registry automatically loads classes only via the system classloader, so you have to explicitly register extractors loaded by other means).


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @HansBrende not yet sorry, I have been overloaded :( I will try tonight.


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    OK folks, so this issue is now available for testing. I can see two immediate issues
    
    1. OpenIE is memory-intensive and can prety easily exhaust >6GB memory for larger jobs. This is an issue if we are running this as a service via any23.org,
    1. There is a considerable amount of bloat on the resulting service artifacts e.g. WAR, tarballs, zip files, etc. See below
    ```
    lmcgibbn@LMC-056430 /usr/local/any23/service/target(ANY23-321) $ ls -lh
    total 13499344
    drwxr-xr-x  5 lmcgibbn  wheel   170B Feb 23 17:47 apache-any23-service-2.2-SNAPSHOT
    -rw-r--r--  1 lmcgibbn  wheel   851M Feb 23 17:50 apache-any23-service-2.2-SNAPSHOT-server-embedded.tar.gz
    -rw-r--r--  1 lmcgibbn  wheel   851M Feb 23 17:50 apache-any23-service-2.2-SNAPSHOT-server-embedded.zip
    -rw-r--r--  1 lmcgibbn  wheel   846M Feb 23 17:48 apache-any23-service-2.2-SNAPSHOT-with-deps.tar.gz
    -rw-r--r--  1 lmcgibbn  wheel   846M Feb 23 17:49 apache-any23-service-2.2-SNAPSHOT-with-deps.zip
    -rw-r--r--  1 lmcgibbn  wheel   784M Feb 23 17:49 apache-any23-service-2.2-SNAPSHOT-without-deps.tar.gz
    -rw-r--r--  1 lmcgibbn  wheel   783M Feb 23 17:48 apache-any23-service-2.2-SNAPSHOT-without-deps.war
    -rw-r--r--  1 lmcgibbn  wheel   784M Feb 23 17:49 apache-any23-service-2.2-SNAPSHOT-without-deps.zip
    -rw-r--r--  1 lmcgibbn  wheel   846M Feb 23 17:47 apache-any23-service-2.2-SNAPSHOT.war
    drwxr-xr-x  2 lmcgibbn  wheel    68B Feb 23 17:47 archive-tmp
    drwxr-xr-x  4 lmcgibbn  wheel   136B Feb 23 17:47 classes
    drwxr-xr-x  3 lmcgibbn  wheel   102B Feb 23 17:47 generated-sources
    drwxr-xr-x  3 lmcgibbn  wheel   102B Feb 23 17:47 generated-test-sources
    drwxr-xr-x  3 lmcgibbn  wheel   102B Feb 23 17:47 maven-archiver
    drwxr-xr-x  3 lmcgibbn  wheel   102B Feb 23 17:47 maven-status
    drwxr-xr-x  4 lmcgibbn  wheel   136B Feb 23 17:47 test-classes
    drwxr-xr-x  4 lmcgibbn  wheel   136B Feb 23 17:47 war-legals
    ```


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @lewismc you said you had some specific lines for debugging?


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    I've messed this PR up now so I will close and reopen another once I have the dependency issue resolved. I'll also provide an update to the documentation as to how dynamic classloading is done per example.  


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @lewismc have you been able to test whether or not my suggested insertion fixes your problem?


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @HansBrende @jgrzebyta can you both please have a look at dynamic plugin loading in this patch? I am at a loss, having tried to debug this with no progress. Thanks in advance, 


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @lewismc my suggestion was to provide a config flag *in addition to* the existing checkbox (the difference being: the config flag would *not* be controlled by the user accessing the webpage--but by the server itself.)
    
    But if the RAM usage is not a problem for any23.org, then there's no need for it. (Although it might still be a good thing to have, e.g., if someone else wants to deploy the same server code to their own server, but disallow OpenIE processing in order to limit RAM usage. I have no idea if that's a realistic use-case though.)


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    Or... in lieu of adding a config flag, it might be nice to just handle `OutOfMemoryError`s gracefully. If an `OutOfMemoryError` is encountered, just skip the OpenIE extraction and move on to the next (or something like that--and feel free to ignore all this advice if it's bad haha.)
    
    In any case, if any23.org can handle 6 GB of memory per request, then I say +1. Cool functionality, and it would be great to be able to use it in the any23 webservice!


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    I logged an issue in JIRA about this. I’ve also been over to see that
    disappointingly there is not Maven artifact available for OpenIE 5.
    I didn’t hear anything back from the OpenIE team either
    https://github.com/dair-iitd/OpenIE-standalone/issues/6
    I suppose one of us could go and contribute the module, however at that
    time, I believe I just started working on the Any23 Service + OpenIE
    integration. As I said it was a while ago.
    
    On Tue, Feb 27, 2018 at 06:34 Hans Brende <no...@github.com> wrote:
    
    > On a slightly unrelated topic, according to this github page
    > <https://github.com/knowitall/openie>, the current version of OpenIE is
    > OpenIE 5, located here <https://github.com/dair-iitd/OpenIE-standalone>,
    > whereas it looks like we are using OpenIE 4.2.6.
    >
    > —
    > You are receiving this because you were mentioned.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/any23/pull/56#issuecomment-368897453>, or mute
    > the thread
    > <https://github.com/notifications/unsubscribe-auth/ABHJl-Ek4C9mQYf7-23oCUzOq8erMT21ks5tZBJegaJpZM4RROYY>
    > .
    >
    -- 
    http://home.apache.org/~lewismc/
    http://people.apache.org/keys/committer/lewismc



---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    On a slightly unrelated topic, according to [this github page](https://github.com/knowitall/openie), the current version of OpenIE is OpenIE 5, located [here](https://github.com/dair-iitd/OpenIE-standalone), whereas it looks like we are using OpenIE 4.2.6.


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    Thanks @HansBrende :)
    Once you have built the Webservice and deployed to Tomcat, you should make sure that you check the **openie** checkbox in the GUI.
    You can set a breakpoint at the [openie conditional toggle on in Servlet.java](https://github.com/lewismc/any23/blob/ANY23-321/service/src/main/java/org/apache/any23/servlet/Servlet.java#L93) which will enable you to see that the parameter is being received on server side. From here, you need to look into how the Any23PluginManager does dynamic classloading.
    Let me know how you get on and I can provide a bit more context as to how the directory openie structure works for dynamic loading.
    Thanks in advance, 


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    I mean, I'm not sure what kind of resources you want your webservice to be using, but 6GB memory sounds like a blocker to me. Using OpenIE with the core api would be opt-in only, right?


---

[GitHub] any23 pull request #56: ANY23-321 Add openie toggle functionality to service

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/any23/pull/56


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    Ah, I see. Well, it may not be the end of the world if we stick with 4.2.6 for now--looks like that version requires less memory anyway.
    
    What do you think about my OOM handling suggestion? I edited [my comment](https://github.com/apache/any23/pull/56#issuecomment-368765010) to provide some sample code.


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    Any comments here folks?


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    Hi @HansBrende your suggestion fixed the code so I have pushed for further review.
    @ansell thank you for the suggestions I think you are absolutely right... I've posted a [question](https://lists.apache.org/thread.html/3dfa2a8fbe170efc88274b639ce0b1032d838046c1c20a361195c330@%3Cusers.maven.apache.org%3E) on users@maven along with my [followup](https://lists.apache.org/thread.html/4ce2d66fe7fff8794f2a08b0c8b3450a1bcbdcb1e40214ab1d1c5059@%3Cusers.maven.apache.org%3E) so hopefully I can keep the service pom clean by obtaining the correct solution.
    I'll post here once I know. Thanks all.



---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @HansBrende 
    
    > What do you think about my OOM handling suggestion? I edited my comment to provide some sample code.
    
    Excellent suggestion, I've accommodated it. I also agree that another setting, possibly within core Any23 Configuration would be good, however i think that this can be added if the exception handling is not sufficient. That would be a different issue.


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    (The flag could even be read at the build-level: for example, you might add a build step that reads your configuration file and modifies the HTML to show or hide the toggle according to whether or not the OpenIE flag is enabled--so as not to make an unnecessary web request. I personally have no experience with building complex build tasks, so I have no idea whether or not this is feasible, or even a good idea. But just a thought.)


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    Yes this is optizonal for the core API and would really only be used to
    demonstrate applications that can be built using Any23’s dynamic plugin
    loading capabilities. This is a good excuse to update he Documentation
    regardless of whether we decide to merge it into master.
    Did you try it locally and see what kind of results you get? It’s pretty
    cool and would nicely complement something like Jena’s full text search
    https://jena.apache.org/documentation/query/text-query.html
    Relistically the triples which are extracted using OpenIE are free text,
    not defined vocabulary hence personally I think it highlights quite a nice
    use case.
    
    On Mon, Feb 26, 2018 at 19:13 Hans Brende <no...@github.com> wrote:
    
    > I mean, I'm not sure what kind of resources you want your webservice to be
    > using, but 6GB memory sounds like a blocker to me. Using OpenIE with the
    > core api would be opt-in only, right?
    >
    > —
    > You are receiving this because you were mentioned.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/any23/pull/56#issuecomment-368732880>, or mute
    > the thread
    > <https://github.com/notifications/unsubscribe-auth/ABHJlxLAaAzyXR3gLi0Gm5QRjEzBUl5Tks5tY3LZgaJpZM4RROYY>
    > .
    >
    -- 
    http://home.apache.org/~lewismc/
    http://people.apache.org/keys/committer/lewismc



---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    I haven't tried it locally, but I agree it's pretty cool functionality and would aptly demonstrate that plugin-loading works.
    
    My main concern, if this will be deployed to any23.org, would be the RAM usage.
    
    One solution might be to add a configuration flag that either enables or disables OpenIE processing (the OpenIE toggle would only be displayed if the flag were enabled)--and disable the flag by default. (Unless whoever's paying for the any23.org web service doesn't mind each request using 6 GB--in that case I guess it wouldn't matter.)


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by jgrzebyta <gi...@git.apache.org>.
Github user jgrzebyta commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @lewismc 
    
    I have: `Failed to execute goal on project apache-any23-service: Could not resolve dependencies for project org.apache.any23:apache-any23-service:war:2.3-SNAPSHOT: Failure to find org.allenai.openie:openie_2.11:jar:4.2.6 in https://svn.apache.org/repos/asf/any23/repo-ext/ was cached in the local repository, resolution will not be reattempted until the update interval of any23-repository-external has elapsed or updates are forced`.
    
    
    IMHO it shouldn't be said in the apache-any23-service pom that the dependency is provided. Provided means it comes together with tomcat.


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by HansBrende <gi...@git.apache.org>.
Github user HansBrende commented on the issue:

    https://github.com/apache/any23/pull/56
  
    @lewismc I really have no idea how to deploy to Tomcat, being a bit of a noob, but have you tried doing something like this in `Servlet.java`?
    
    ```
    if (loadedJars) { 
        ExtractorRegistry r = ExtractorRegistryImpl.getInstance();
        pManager.getExtractors().forEachRemaining(r::register);
        ...
    }
    ```


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by ansell <gi...@git.apache.org>.
Github user ansell commented on the issue:

    https://github.com/apache/any23/pull/56
  
    I don't have time right now to debug this or work on its design. It isn't a pattern I have worked on in the past. All ``ServiceLoader`` projects that I have worked on in the past either have all of the service Factory objects initialised as startup, or they do periodic scans, but always create and initialise the service Factory objects whenever they are discovered. 
    
    Is this issue attempting to delay/extract the OpenIE initialisation by delaying/postponing the Factory initialisation? If so, the focus could possibly be on making the OpenIE initialisation lazy to only occur when the Factory is actually required/used to create an object instance.
    
    A more configurable lifecycle possibly requires OSGi, which I again have absolutely no experience with, but from the little understanding each time I look, it appears to allow user driven initialisation (push rather than pull, if that makes sense).


---

[GitHub] any23 issue #56: ANY23-321 Add openie toggle functionality to service

Posted by lewismc <gi...@git.apache.org>.
Github user lewismc commented on the issue:

    https://github.com/apache/any23/pull/56
  
    > My main concern, if this will be deployed to any23.org, would be the RAM usage.
    
    We can always request more RAM for the dedicated VM powering any23.org
    
    > One solution might be to add a configuration flag that either enables or disables OpenIE processing (the OpenIE toggle would only be displayed if the flag were enabled)--and disable the flag by default. 
    
    This is exactly whats provided in the patch. I guessed the it should be disabled by default. You can see examples of the toggle functionality in the snapshot's below
    
    <img width="643" alt="screen shot 2018-02-26 at 10 00 44 pm" src="https://user-images.githubusercontent.com/1165719/36713077-a68e77ea-1b40-11e8-9a7c-68fa7aaa73b9.png">
    
    <img width="559" alt="screen shot 2018-02-26 at 10 00 59 pm" src="https://user-images.githubusercontent.com/1165719/36713083-aa8dc080-1b40-11e8-8590-4084b5996c0f.png">



---