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/05/25 12:34:11 UTC
[GitHub] incubator-metron pull request: METRON-100 GeoIP errors out silentl...
GitHub user cestella opened a pull request:
https://github.com/apache/incubator-metron/pull/134
METRON-100 GeoIP errors out silently in vagrant
When we transitioned from passing the value to the adapters to passing a CacheKey object, we never adjusted the SQL statements. Also, this error may cause downstream issues whereby messages error out because enrichments are short circuited if one fails and the source.type never gets properly set.
The JIRA is wrong, this bug is actually present in every environment and prevents GeoIP enrichment from working.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cestella/incubator-metron enrichment_robustness
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-metron/pull/134.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 #134
----
commit 6fafdcf66df4a1d1df21672f2644fd00953e75f9
Author: cstella <ce...@gmail.com>
Date: 2016-05-24T22:26:37Z
Making the bolts and adapters slightly more robust so that failures for a single enrichment do not exception out and kill the topology.
commit 865cbc942ceff92e47ca83e664ae6bc9c3f3cd5a
Author: cstella <ce...@gmail.com>
Date: 2016-05-25T00:34:25Z
Continued robustness improvements.
----
---
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: METRON-100 GeoIP errors out silentl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-metron/pull/134
---
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: METRON-100 GeoIP errors out silentl...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the pull request:
https://github.com/apache/incubator-metron/pull/134#issuecomment-221924679
Good catch. Looks great. +1
---
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: METRON-100 GeoIP errors out silentl...
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/134#discussion_r64741859
--- Diff: metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/adapters/jdbc/JdbcAdapter.java ---
@@ -38,6 +39,27 @@
private JdbcConfig config;
private String host;
+ protected boolean isConnectionClosed() {
+ boolean isClosed = statement == null || connection == null;
+ if(!isClosed) {
+ try {
+ isClosed = statement.isClosed() || connection.isClosed();
+ } catch (SQLException e) {
+ _LOG.error("Unable to maintain open JDBC connection: " + e.getMessage(), e);
+ isClosed = true;
+ }
+ }
+ return isClosed;
+ }
+
+ protected boolean resetConnectionIfNecessary() {
+ if(isConnectionClosed())
+ {
--- End diff --
Sorry, nit-picky, but we have a new line between the if and the '{' which you don't do elsewhere in this code. Would be nice to stick to one convention.
We need to enhance our code style checks. Hard to catch and enforce manually.
---
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: METRON-100 GeoIP errors out silentl...
Posted by dlyle65535 <gi...@git.apache.org>.
Github user dlyle65535 commented on the pull request:
https://github.com/apache/incubator-metron/pull/134#issuecomment-221683999
+1!
Thanks
---
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: METRON-100 GeoIP errors out silentl...
Posted by AromalAS <gi...@git.apache.org>.
Github user AromalAS commented on the pull request:
https://github.com/apache/incubator-metron/pull/134#issuecomment-221927743
Tested with the latest code changes, ..Good Fix
---
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: METRON-100 GeoIP errors out silentl...
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/134#discussion_r64742580
--- Diff: metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/adapters/jdbc/JdbcAdapter.java ---
@@ -38,6 +39,27 @@
private JdbcConfig config;
private String host;
+ protected boolean isConnectionClosed() {
+ boolean isClosed = statement == null || connection == null;
+ if(!isClosed) {
+ try {
+ isClosed = statement.isClosed() || connection.isClosed();
+ } catch (SQLException e) {
+ _LOG.error("Unable to maintain open JDBC connection: " + e.getMessage(), e);
+ isClosed = true;
+ }
+ }
+ return isClosed;
+ }
+
+ protected boolean resetConnectionIfNecessary() {
+ if(isConnectionClosed())
+ {
--- End diff --
So, I'll definitely make the class consistently K&R (I must've fat-fingered the newline). We don't have a coherent style at the moment in this respect. I prefer K&R bracing, so you'll see that in my code, but yeah, I agree, we need to correct style as we see it.
Also, don't worry about nitpicking..we all have to live in this code. :)
---
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.
---