You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by cestella <gi...@git.apache.org> on 2016/08/26 17:23:18 UTC
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
GitHub user cestella opened a pull request:
https://github.com/apache/incubator-metron/pull/233
METRON-396: Make Stellar function resolution happen via an annotation and classpath search
At the moment, the only way to add a stellar function is to implement StellarFunction and add the instance to the StellarFunctions enum. This is deficient in a few ways:
* It requires you to modify Metron code to add a stellar capability
* It requires you to make metron-common depend on your project
* There is no way to enforce documentation of the function
Instead, we should create an annotation which will define:
* function name
* namespace
* description
* List of parameters
* return documentation
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cestella/incubator-metron METRON-396
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-metron/pull/233.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 #233
----
commit 465d37e64861a57dc878f13e5c0685ab31ab3b73
Author: cstella <ce...@gmail.com>
Date: 2016-08-26T17:21:13Z
METRON-396: Make Stellar function resolution happen via an annotation and classpath search
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/233
FYI - Your branch needs merged with master before we can push this one along any further. Not sure if you've missed that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-metron/pull/233
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/233
Yeah, I see what you're saying; that's a good catch. You are doing some testing for initialization and you want to uninitialize. One thing we could do is provide a function in the FunctionResolver to retrieve the StellarFunction given a name and you could set your initialized boolean to false when you wanted to selectively reset.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/233#discussion_r77065193
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/FunctionResolverSingleton.java ---
@@ -0,0 +1,206 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.dsl;
+
+import com.google.common.base.Joiner;
+import org.reflections.Reflections;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class FunctionResolverSingleton implements FunctionResolver {
+ protected static final Logger LOG = LoggerFactory.getLogger(FunctionResolverSingleton.class);
+ private final Map<String, StellarFunctionInfo> functions = new HashMap<>();
+ private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+ private static final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private static FunctionResolverSingleton INSTANCE = new FunctionResolverSingleton();
+
+ private FunctionResolverSingleton() {}
+
+ public static FunctionResolver getInstance() {
+ return INSTANCE;
+ }
+
+
+
+ @Override
+ public Iterable<StellarFunctionInfo> getFunctionInfo() {
+ return _getFunctions().values();
+ }
+
+ @Override
+ public Iterable<String> getFunctions() {
+ return _getFunctions().keySet();
+ }
+
+ @Override
+ public void initialize(Context context) {
+ //forces a load of the stellar functions.
+ _getFunctions();
+ }
+
+ /**
+ * This allows the lazy loading of the functions. We do not want to take a multi-second hit to analyze the full classpath
+ * every time a unit test starts up. That would cause the runtime of things to blow right up. Instead, we only want
+ * to take the hit if a function is actually called from a stellar expression.
+ *
+ *
+ * @return The map of the stellar functions that we found on the classpath indexed by fully qualified name
+ */
+ private Map<String, StellarFunctionInfo> _getFunctions() {
+ /*
+ * Because we are not doing this in a static block and because this object is a singleton we have to concern ourselves with
+ * the possiblity that two threads are calling this function at the same time. Normally, I would consider just making the
+ * function synchronized, but since every stellar statement which uses a function will be here, I wanted to distinguish
+ * between read locks (that happen often and are quickly resolved) and write locks (which should happen at initialization time).
+ */
+ lock.readLock().lock();
+ try {
+ if (isInitialized.get()) {
+ return functions;
+ }
+ }
+ finally {
+ lock.readLock().unlock();
+ }
+ //we should VERY rarely get here.
+ lock.writeLock().lock();
+ try {
+ //I have to check again because two threads or more could be waiting at the lock statement. The loser threads
+ //shouldn't reinitialize.
+ if(!isInitialized.get()) {
+ loadFunctions(functions);
+ isInitialized.set(true);
+ }
+ return functions;
+ }
+ finally {
+ lock.writeLock().unlock();
+ }
+ }
+ /**
+ * Applies this function to the given argument.
+ *
+ * @param s the function argument
+ * @return the function result
+ */
+ @Override
+ public StellarFunction apply(String s) {
+ StellarFunctionInfo ret = _getFunctions().get(s);
+ if(ret == null) {
+ throw new IllegalStateException("Unable to resolve function " + s);
+ }
+ return ret.getFunction();
+ }
+
+ private void loadFunctions(final Map<String, StellarFunctionInfo> ret) {
+ try {
+ ClassLoader classLoader = getClass().getClassLoader();
+ Reflections reflections = new Reflections(new ConfigurationBuilder().setUrls(effectiveClassPathUrls(classLoader)));
--- End diff --
It just pains me to see us spending so much time searching jars in the classpath that we know will never contain any Stellar functions. :) It currently searches all of the dependencies like HBase, Storm, Mockito, Jackson, etc. It is all of those that give us the 5-8 second delay.
An easy way to do this would be to leverage the `Context` functionality that already exists.
1. Create `Capabilities.STELLAR_PATH` that is either a colon-separated list of paths to search (like the Java CLASSPATH) or a regular expression of paths to search (ex; `.*metron-common.*`).
2. Update `FunctionResolverSingleton.initialize(Context)` to extract the new capability, if it exists. If the capability does not exist, or `initialize` is not called as part of lazy initialization, then choose a sensible default, which could be either:
* Include All - The current behavior now which searches the entire classpath
* Metron Only - Include only Metron jars known to have Stellar functions; `.*-metron.*`
* Something in between like "anything that looks like `.*-stellar-.*` or `.*-metron.*`
3. Update `FunctionResolverSingleton.loadFunctions` so that it only searches the paths defined by the variable from step 2.
Even if we default to "Include All" this approach at least lets me update unit and integration tests to confine the breadth of the classpath search so that we don't introduce unnecessary delay running them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/233#discussion_r76918718
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/FunctionResolverSingleton.java ---
@@ -0,0 +1,206 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.dsl;
+
+import com.google.common.base.Joiner;
+import org.reflections.Reflections;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class FunctionResolverSingleton implements FunctionResolver {
+ protected static final Logger LOG = LoggerFactory.getLogger(FunctionResolverSingleton.class);
+ private final Map<String, StellarFunctionInfo> functions = new HashMap<>();
+ private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+ private static final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private static FunctionResolverSingleton INSTANCE = new FunctionResolverSingleton();
+
+ private FunctionResolverSingleton() {}
+
+ public static FunctionResolver getInstance() {
+ return INSTANCE;
+ }
+
+
+
+ @Override
+ public Iterable<StellarFunctionInfo> getFunctionInfo() {
+ return _getFunctions().values();
+ }
+
+ @Override
+ public Iterable<String> getFunctions() {
+ return _getFunctions().keySet();
+ }
+
+ @Override
+ public void initialize(Context context) {
+ //forces a load of the stellar functions.
+ _getFunctions();
+ }
+
+ /**
+ * This allows the lazy loading of the functions. We do not want to take a multi-second hit to analyze the full classpath
+ * every time a unit test starts up. That would cause the runtime of things to blow right up. Instead, we only want
+ * to take the hit if a function is actually called from a stellar expression.
+ *
+ *
+ * @return The map of the stellar functions that we found on the classpath indexed by fully qualified name
+ */
+ private Map<String, StellarFunctionInfo> _getFunctions() {
+ /*
+ * Because we are not doing this in a static block and because this object is a singleton we have to concern ourselves with
+ * the possiblity that two threads are calling this function at the same time. Normally, I would consider just making the
+ * function synchronized, but since every stellar statement which uses a function will be here, I wanted to distinguish
+ * between read locks (that happen often and are quickly resolved) and write locks (which should happen at initialization time).
+ */
+ lock.readLock().lock();
+ try {
+ if (isInitialized.get()) {
+ return functions;
+ }
+ }
+ finally {
+ lock.readLock().unlock();
+ }
+ //we should VERY rarely get here.
+ lock.writeLock().lock();
+ try {
+ //I have to check again because two threads or more could be waiting at the lock statement. The loser threads
+ //shouldn't reinitialize.
+ if(!isInitialized.get()) {
+ loadFunctions(functions);
+ isInitialized.set(true);
+ }
+ return functions;
+ }
+ finally {
+ lock.writeLock().unlock();
+ }
+ }
+ /**
+ * Applies this function to the given argument.
+ *
+ * @param s the function argument
+ * @return the function result
+ */
+ @Override
+ public StellarFunction apply(String s) {
+ StellarFunctionInfo ret = _getFunctions().get(s);
+ if(ret == null) {
+ throw new IllegalStateException("Unable to resolve function " + s);
+ }
+ return ret.getFunction();
+ }
+
+ private void loadFunctions(final Map<String, StellarFunctionInfo> ret) {
+ try {
+ ClassLoader classLoader = getClass().getClassLoader();
+ Reflections reflections = new Reflections(new ConfigurationBuilder().setUrls(effectiveClassPathUrls(classLoader)));
--- End diff --
Would it make sense to introduce some kind of 'import' like functionality? Instead of searching the entire classpath, the user would specifically define the locations of Stellar functions that they want to use.
I am curious if a search of the entire classpath will be tenable in all situations. In terms of performance, its border line acceptable now, but how might that behave differently in other environments; could it be even slower?
A full classpath search may also increase the chance of unintended naming collisions. And in the case of a collision, the definition that wins is non-deterministic. That would cause a major migraine to troubleshoot.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/233
Due to the static initialization and use of a singleton for the function resolver, this was a bit difficult to trace. We should think about making this easier to grok as a follow-on to this PR. This would be painful for a new user creating their own functions. I don't personally have a solution right now, but need to noodle on it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/233
Ok, this should be a transparent change to the topology. Things should continue to function as expected. I also verified that this did not break pcap's `query` filter.
As such, a reasonable test plan is to spin the topologies up, ensure that data flows through.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/233#discussion_r77121424
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/FunctionResolverSingleton.java ---
@@ -0,0 +1,206 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.dsl;
+
+import com.google.common.base.Joiner;
+import org.reflections.Reflections;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class FunctionResolverSingleton implements FunctionResolver {
+ protected static final Logger LOG = LoggerFactory.getLogger(FunctionResolverSingleton.class);
+ private final Map<String, StellarFunctionInfo> functions = new HashMap<>();
+ private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+ private static final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private static FunctionResolverSingleton INSTANCE = new FunctionResolverSingleton();
+
+ private FunctionResolverSingleton() {}
+
+ public static FunctionResolver getInstance() {
+ return INSTANCE;
+ }
+
+
+
+ @Override
+ public Iterable<StellarFunctionInfo> getFunctionInfo() {
+ return _getFunctions().values();
+ }
+
+ @Override
+ public Iterable<String> getFunctions() {
+ return _getFunctions().keySet();
+ }
+
+ @Override
+ public void initialize(Context context) {
+ //forces a load of the stellar functions.
+ _getFunctions();
+ }
+
+ /**
+ * This allows the lazy loading of the functions. We do not want to take a multi-second hit to analyze the full classpath
+ * every time a unit test starts up. That would cause the runtime of things to blow right up. Instead, we only want
+ * to take the hit if a function is actually called from a stellar expression.
+ *
+ *
+ * @return The map of the stellar functions that we found on the classpath indexed by fully qualified name
+ */
+ private Map<String, StellarFunctionInfo> _getFunctions() {
+ /*
+ * Because we are not doing this in a static block and because this object is a singleton we have to concern ourselves with
+ * the possiblity that two threads are calling this function at the same time. Normally, I would consider just making the
+ * function synchronized, but since every stellar statement which uses a function will be here, I wanted to distinguish
+ * between read locks (that happen often and are quickly resolved) and write locks (which should happen at initialization time).
+ */
+ lock.readLock().lock();
+ try {
+ if (isInitialized.get()) {
+ return functions;
+ }
+ }
+ finally {
+ lock.readLock().unlock();
+ }
+ //we should VERY rarely get here.
+ lock.writeLock().lock();
+ try {
+ //I have to check again because two threads or more could be waiting at the lock statement. The loser threads
+ //shouldn't reinitialize.
+ if(!isInitialized.get()) {
+ loadFunctions(functions);
+ isInitialized.set(true);
+ }
+ return functions;
+ }
+ finally {
+ lock.writeLock().unlock();
+ }
+ }
+ /**
+ * Applies this function to the given argument.
+ *
+ * @param s the function argument
+ * @return the function result
+ */
+ @Override
+ public StellarFunction apply(String s) {
+ StellarFunctionInfo ret = _getFunctions().get(s);
+ if(ret == null) {
+ throw new IllegalStateException("Unable to resolve function " + s);
+ }
+ return ret.getFunction();
+ }
+
+ private void loadFunctions(final Map<String, StellarFunctionInfo> ret) {
+ try {
+ ClassLoader classLoader = getClass().getClassLoader();
+ Reflections reflections = new Reflections(new ConfigurationBuilder().setUrls(effectiveClassPathUrls(classLoader)));
--- End diff --
I think that's a fine addition as a separate JIRA. At the moment, this is executed once per topology start, so I'm ok with this default implementation. Also, keep in mind that there are edge cases here, specifically around jars with other jars referenced in their manifest that the Reflections API is handling under the hood. It's a worthy task, but one that should be a follow-on to this task.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/233
Ok, merged master in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/233
I have been tracing an issue caused by static initialization. I have a Stellar function that uses values within the Context to perform initialization. I have multiple tests in the same class; test1(), test2(), etc. In each of these tests, I need to initialize the function with different values in the Context.
If I run each test individually, everything works just fine, but if I run all the tests together the tests fail. The problem is that the functions don't actually re-initialize in test2() after they have already been initialized in test1().
I think I need a way to manually clear the functions cached in `FunctionResolverSingleton._getFunctions()` that I would call before running each unit test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/233#discussion_r76918935
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/FunctionResolverSingleton.java ---
@@ -0,0 +1,206 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.dsl;
+
+import com.google.common.base.Joiner;
+import org.reflections.Reflections;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class FunctionResolverSingleton implements FunctionResolver {
+ protected static final Logger LOG = LoggerFactory.getLogger(FunctionResolverSingleton.class);
+ private final Map<String, StellarFunctionInfo> functions = new HashMap<>();
+ private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+ private static final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private static FunctionResolverSingleton INSTANCE = new FunctionResolverSingleton();
+
+ private FunctionResolverSingleton() {}
+
+ public static FunctionResolver getInstance() {
+ return INSTANCE;
+ }
+
+
+
+ @Override
+ public Iterable<StellarFunctionInfo> getFunctionInfo() {
+ return _getFunctions().values();
+ }
+
+ @Override
+ public Iterable<String> getFunctions() {
+ return _getFunctions().keySet();
+ }
+
+ @Override
+ public void initialize(Context context) {
+ //forces a load of the stellar functions.
+ _getFunctions();
+ }
+
+ /**
+ * This allows the lazy loading of the functions. We do not want to take a multi-second hit to analyze the full classpath
+ * every time a unit test starts up. That would cause the runtime of things to blow right up. Instead, we only want
+ * to take the hit if a function is actually called from a stellar expression.
+ *
+ *
+ * @return The map of the stellar functions that we found on the classpath indexed by fully qualified name
+ */
+ private Map<String, StellarFunctionInfo> _getFunctions() {
+ /*
+ * Because we are not doing this in a static block and because this object is a singleton we have to concern ourselves with
+ * the possiblity that two threads are calling this function at the same time. Normally, I would consider just making the
+ * function synchronized, but since every stellar statement which uses a function will be here, I wanted to distinguish
+ * between read locks (that happen often and are quickly resolved) and write locks (which should happen at initialization time).
+ */
+ lock.readLock().lock();
+ try {
+ if (isInitialized.get()) {
+ return functions;
+ }
+ }
+ finally {
+ lock.readLock().unlock();
+ }
+ //we should VERY rarely get here.
+ lock.writeLock().lock();
+ try {
+ //I have to check again because two threads or more could be waiting at the lock statement. The loser threads
+ //shouldn't reinitialize.
+ if(!isInitialized.get()) {
+ loadFunctions(functions);
+ isInitialized.set(true);
+ }
+ return functions;
+ }
+ finally {
+ lock.writeLock().unlock();
+ }
+ }
+ /**
+ * Applies this function to the given argument.
+ *
+ * @param s the function argument
+ * @return the function result
+ */
+ @Override
+ public StellarFunction apply(String s) {
+ StellarFunctionInfo ret = _getFunctions().get(s);
+ if(ret == null) {
+ throw new IllegalStateException("Unable to resolve function " + s);
+ }
+ return ret.getFunction();
+ }
+
+ private void loadFunctions(final Map<String, StellarFunctionInfo> ret) {
+ try {
+ ClassLoader classLoader = getClass().getClassLoader();
+ Reflections reflections = new Reflections(new ConfigurationBuilder().setUrls(effectiveClassPathUrls(classLoader)));
--- End diff --
So the class path search happens once in the prepare phase of storm and
name collision results in an initialization failure.
I think it is quite tenable now, but we can consider an import statement as
a follow-on.
On Wed, Aug 31, 2016 at 13:24 Nick Allen <no...@github.com> wrote:
> In
> metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/FunctionResolverSingleton.java
> <https://github.com/apache/incubator-metron/pull/233#discussion_r76918718>
> :
>
> > + * @param s the function argument
> > + * @return the function result
> > + */
> > + @Override
> > + public StellarFunction apply(String s) {
> > + StellarFunctionInfo ret = _getFunctions().get(s);
> > + if(ret == null) {
> > + throw new IllegalStateException("Unable to resolve function " + s);
> > + }
> > + return ret.getFunction();
> > + }
> > +
> > + private void loadFunctions(final Map<String, StellarFunctionInfo> ret) {
> > + try {
> > + ClassLoader classLoader = getClass().getClassLoader();
> > + Reflections reflections = new Reflections(new ConfigurationBuilder().setUrls(effectiveClassPathUrls(classLoader)));
>
> Would it make sense to introduce some kind of 'import' like functionality?
> Instead of searching the entire classpath, the user would specifically
> define the locations of Stellar functions that they want to use.
>
> I am curious if a search of the entire classpath will be tenable in all
> situations. In terms of performance, its border line acceptable now, but
> how might that behave differently in other environments; could it be even
> slower?
>
> A full classpath search may also increase the chance of unintended naming
> collisions. And in the case of a collision, the definition that wins is
> non-deterministic. That would cause a major migraine to troubleshoot.
>
> \u2014
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-metron/pull/233/files/465d37e64861a57dc878f13e5c0685ab31ab3b73#r76918718>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAg-xwYuCNqxkCrjq-LF2D4AK5lSac35ks5qlPP1gaJpZM4JuSgL>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/233
Yeah getting to that; in a bit of a Internet connection blackhole
On Fri, Sep 2, 2016 at 07:43 Nick Allen <no...@github.com> wrote:
> FYI - Your branch needs merged with master before we can push this one
> along any further. Not sure if you've missed that.
>
> \u2014
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-metron/pull/233#issuecomment-244394663>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAg-x93Eu-DCniqVEpUvJTJ7giwO8l2Yks5qmDX8gaJpZM4JuSgL>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/233
I had to address this issue as part of some other work. It is included in #242.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #233: METRON-396: Make Stellar function resolu...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/233#discussion_r77002294
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/FunctionResolverSingleton.java ---
@@ -0,0 +1,206 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.dsl;
+
+import com.google.common.base.Joiner;
+import org.reflections.Reflections;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class FunctionResolverSingleton implements FunctionResolver {
+ protected static final Logger LOG = LoggerFactory.getLogger(FunctionResolverSingleton.class);
+ private final Map<String, StellarFunctionInfo> functions = new HashMap<>();
+ private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+ private static final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private static FunctionResolverSingleton INSTANCE = new FunctionResolverSingleton();
+
+ private FunctionResolverSingleton() {}
+
+ public static FunctionResolver getInstance() {
+ return INSTANCE;
+ }
+
+
+
+ @Override
+ public Iterable<StellarFunctionInfo> getFunctionInfo() {
+ return _getFunctions().values();
+ }
+
+ @Override
+ public Iterable<String> getFunctions() {
+ return _getFunctions().keySet();
+ }
+
+ @Override
+ public void initialize(Context context) {
+ //forces a load of the stellar functions.
+ _getFunctions();
+ }
+
+ /**
+ * This allows the lazy loading of the functions. We do not want to take a multi-second hit to analyze the full classpath
+ * every time a unit test starts up. That would cause the runtime of things to blow right up. Instead, we only want
+ * to take the hit if a function is actually called from a stellar expression.
+ *
+ *
+ * @return The map of the stellar functions that we found on the classpath indexed by fully qualified name
+ */
+ private Map<String, StellarFunctionInfo> _getFunctions() {
+ /*
+ * Because we are not doing this in a static block and because this object is a singleton we have to concern ourselves with
+ * the possiblity that two threads are calling this function at the same time. Normally, I would consider just making the
+ * function synchronized, but since every stellar statement which uses a function will be here, I wanted to distinguish
+ * between read locks (that happen often and are quickly resolved) and write locks (which should happen at initialization time).
+ */
+ lock.readLock().lock();
+ try {
+ if (isInitialized.get()) {
+ return functions;
+ }
+ }
+ finally {
+ lock.readLock().unlock();
+ }
+ //we should VERY rarely get here.
+ lock.writeLock().lock();
+ try {
+ //I have to check again because two threads or more could be waiting at the lock statement. The loser threads
+ //shouldn't reinitialize.
+ if(!isInitialized.get()) {
+ loadFunctions(functions);
+ isInitialized.set(true);
+ }
+ return functions;
+ }
+ finally {
+ lock.writeLock().unlock();
+ }
+ }
+ /**
+ * Applies this function to the given argument.
+ *
+ * @param s the function argument
+ * @return the function result
+ */
+ @Override
+ public StellarFunction apply(String s) {
+ StellarFunctionInfo ret = _getFunctions().get(s);
+ if(ret == null) {
+ throw new IllegalStateException("Unable to resolve function " + s);
+ }
+ return ret.getFunction();
+ }
+
+ private void loadFunctions(final Map<String, StellarFunctionInfo> ret) {
+ try {
+ ClassLoader classLoader = getClass().getClassLoader();
+ Reflections reflections = new Reflections(new ConfigurationBuilder().setUrls(effectiveClassPathUrls(classLoader)));
--- End diff --
Yes, I think it would be a good follow-on. I really just bring it up as a discussion point to think about. Good idea, bad idea?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #233: METRON-396: Make Stellar function resolution ha...
Posted by dlyle65535 <gi...@git.apache.org>.
Github user dlyle65535 commented on the issue:
https://github.com/apache/incubator-metron/pull/233
+1, good deal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---