You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by tibkiss <gi...@git.apache.org> on 2017/02/12 10:57:52 UTC
[GitHub] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
GitHub user tibkiss opened a pull request:
https://github.com/apache/storm/pull/1937
STORM-2355: Storm-HDFS: inotify support
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tibkiss/storm feature/inotify_in_hdfsspout-1.x-branch
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1937.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 #1937
----
commit dc9aec20b567e752adfe0c1e7c864d86a8d92e68
Author: Tibor Kiss <ti...@gmail.com>
Date: 2017-02-12T09:56:01Z
STORM-2355: Storm-HDFS: inotify support (initial version)
Change-Id: I5e8a81738ff59afb8073a75c604bde2c1ef5b8d1
----
---
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] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1937#discussion_r103069596
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/spout/HdfsSpout.java ---
@@ -596,9 +622,16 @@ private FileReader pickNextFile() {
}
// 2) If no abandoned files, then pick oldest file in sourceDirPath, lock it and rename it
- Collection<Path> listing = HdfsUtils.listFilesByModificationTime(hdfs, sourceDirPath, 0);
-
- for (Path file : listing) {
+ try {
+ hdfsDirectoryMonitor.update();
+ } catch (Exception e) {
+ LOG.error("Unable to update source dir: " + sourceDirPath, e);
+ return null;
+ }
+ for (Path file : hdfsDirectoryMonitor) {
+ if (Path.getPathWithoutSchemeAndAuthority(file).toString().startsWith(lockDirPath.toString())) {
--- End diff --
we can merge these two if blocks into single if block.
---
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] storm issue #1937: STORM-2355: Storm-HDFS: inotify support
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1937
@tibkiss on the JIRA I had a few questions to get clarity around this.
That would help get a perspective to evaluate this. Could you please respond to 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] storm issue #1937: STORM-2355: Storm-HDFS: inotify support
Posted by roshannaik <gi...@git.apache.org>.
Github user roshannaik commented on the issue:
https://github.com/apache/storm/pull/1937
@tibkiss thanks for adding the details on the jira. that is useful. i have started looking into 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] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1937#discussion_r103069587
--- Diff: external/storm-hdfs/src/test/java/org/apache/storm/hdfs/spout/TestHdfsSpout.java ---
@@ -44,21 +44,22 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
--- End diff --
same here.
---
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] storm issue #1937: STORM-2355: Storm-HDFS: inotify support
Posted by tibkiss <gi...@git.apache.org>.
Github user tibkiss commented on the issue:
https://github.com/apache/storm/pull/1937
bump
---
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] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1937#discussion_r103069490
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/common/HdfsDirectoryMonitor.java ---
@@ -0,0 +1,186 @@
+/**
+ * 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
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
+ * 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.storm.hdfs.common;
+
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdfs.DFSClient;
+import org.apache.hadoop.hdfs.DFSInotifyEventInputStream;
+import org.apache.hadoop.hdfs.client.HdfsAdmin;
+import org.apache.hadoop.hdfs.inotify.Event;
+import org.apache.hadoop.hdfs.inotify.EventBatch;
+import org.apache.hadoop.hdfs.inotify.MissingEventsException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.*;
--- End diff --
avoid wild-card imports.
---
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] storm issue #1937: STORM-2355: Storm-HDFS: inotify support
Posted by tibkiss <gi...@git.apache.org>.
Github user tibkiss commented on the issue:
https://github.com/apache/storm/pull/1937
@harshach : Updated this PR based on your review.
---
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] storm issue #1937: STORM-2355: Storm-HDFS: inotify support
Posted by tibkiss <gi...@git.apache.org>.
Github user tibkiss commented on the issue:
https://github.com/apache/storm/pull/1937
@roshannaik : I've replied to your questions on the JIRA by extending the description issue description. Please check it out and let me know if you have further questions. 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] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1937#discussion_r103069609
--- Diff: pom.xml ---
@@ -234,7 +234,7 @@
<clojure.tools.cli.version>0.2.4</clojure.tools.cli.version>
<disruptor.version>3.3.2</disruptor.version>
<jgrapht.version>0.9.0</jgrapht.version>
- <guava.version>16.0.1</guava.version>
+ <guava.version>14.0.1</guava.version>
--- End diff --
any reason to revert to a older version of guava
---
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] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1937
---
[GitHub] storm pull request #1937: STORM-2355: Storm-HDFS: inotify support
Posted by tibkiss <gi...@git.apache.org>.
Github user tibkiss commented on a diff in the pull request:
https://github.com/apache/storm/pull/1937#discussion_r103071553
--- Diff: pom.xml ---
@@ -234,7 +234,7 @@
<clojure.tools.cli.version>0.2.4</clojure.tools.cli.version>
<disruptor.version>3.3.2</disruptor.version>
<jgrapht.version>0.9.0</jgrapht.version>
- <guava.version>16.0.1</guava.version>
+ <guava.version>14.0.1</guava.version>
--- End diff --
Hdfs dependency hell story: hadoop depends on 14.0.1 guava. If guava 16 is used & inotify calls are made the following exception occurs:
`org.apache.hadoop.ipc.RemoteException: com.google.common.collect.ComparisonChain.compare(ZZ)Lcom/google/common/collect/ComparisonChain;
at org.apache.hadoop.hdfs.server.namenode.JournalSet$1.compare(JournalSet.java:67)
at org.apache.hadoop.hdfs.server.namenode.JournalSet$1.compare(JournalSet.java:62)
at java.util.TimSort.countRunAndMakeAscending(TimSort.java:324)
at java.util.TimSort.sort(TimSort.java:189)
at java.util.TimSort.sort(TimSort.java:173)
at java.util.Arrays.sort(Arrays.java:659)
at java.util.Collections.sort(Collections.java:217)
at org.apache.hadoop.hdfs.server.namenode.JournalSet.chainAndMakeRedundantStreams(JournalSet.java:333)
at org.apache.hadoop.hdfs.server.namenode.JournalSet.selectInputStreams(JournalSet.java:284)
at org.apache.hadoop.hdfs.server.namenode.FSEditLog.selectInputStreams(FSEditLog.java:1508)
at org.apache.hadoop.hdfs.server.namenode.FSEditLog.selectInputStreams(FSEditLog.java:1532)
at org.apache.hadoop.hdfs.server.namenode.NameNodeRpcServer.getEditsFromTxid(NameNodeRpcServer.java:1997)
at org.apache.hadoop.hdfs.protocolPB.ClientNamenodeProtocolServerSideTranslatorPB.getEditsFromTxid(ClientNamenodeProtocolServerSideTranslatorPB.java:1496)
at org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos$ClientNamenodeProtocol$2.callBlockingMethod(ClientNamenodeProtocolProtos.java)
at org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:616)
at org.apache.hadoop.ipc.RPC$Server.call(RPC.java:982)
at org.apache.hadoop.ipc.Server$Handler$1.run(Server.java:2049)
at org.apache.hadoop.ipc.Server$Handler$1.run(Server.java:2045)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:415)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1698)
at org.apache.hadoop.ipc.Server$Handler.run(Server.java:2045)
at org.apache.hadoop.ipc.Client.call(Client.java:1475)
at org.apache.hadoop.ipc.Client.call(Client.java:1412)
at org.apache.hadoop.ipc.ProtobufRpcEngine$Invoker.invoke(ProtobufRpcEngine.java:229)
at com.sun.proxy.$Proxy37.getEditsFromTxid(Unknown Source)
at org.apache.hadoop.hdfs.protocolPB.ClientNamenodeProtocolTranslatorPB.getEditsFromTxid(ClientNamenodeProtocolTranslatorPB.java:1511)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.apache.hadoop.io.retry.RetryInvocationHandler.invokeMethod(RetryInvocationHandler.java:191)
at org.apache.hadoop.io.retry.RetryInvocationHandler.invoke(RetryInvocationHandler.java:102)
at com.sun.proxy.$Proxy38.getEditsFromTxid(Unknown Source)
at org.apache.hadoop.hdfs.DFSInotifyEventInputStream.poll(DFSInotifyEventInputStream.java:111)
at org.apache.storm.hdfs.common.HdfsUtils.listNewFiles(HdfsUtils.java:94)
at org.apache.storm.hdfs.spout.TestHdfsSpout.testListNewFiles(TestHdfsSpout.java:549)`
---
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] storm issue #1937: STORM-2355: Storm-HDFS: inotify support
Posted by tibkiss <gi...@git.apache.org>.
Github user tibkiss commented on the issue:
https://github.com/apache/storm/pull/1937
Thanks for the comments. I'll be OoO next week, will address them beginning of March.
---
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.
---