[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user arunmahadevan commented on a diff in the pull request: https://github.com/apache/storm/pull/2773#discussion_r216796216 --- Diff: storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java --- @@ -191,6 +192,8 @@ public static boolean downloadUpdatedBlob(Map conf, BlobStore bl out.close(); } isSuccess = true; +} catch(FileNotFoundException fnf) { --- End diff -- Btw, theres a race condition where the topology gets killed while the blob download is going on and apparently it throws a FNF exception here when invoking out.close(). ---
[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2773#discussion_r206231780 --- Diff: storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java --- @@ -191,6 +192,8 @@ public static boolean downloadUpdatedBlob(Map conf, BlobStore bl out.close(); } isSuccess = true; +} catch(FileNotFoundException fnf) { --- End diff -- I agree with @HeartSaVioR here. ---
[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/2773#discussion_r205973135 --- Diff: storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java --- @@ -197,7 +198,7 @@ public void run() { throw new RuntimeException(e); } } -}, 0, ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS))); +}, 0, ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS))*1000); --- End diff -- Nice catch! ---
[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/2773#discussion_r205973204 --- Diff: storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java --- @@ -191,6 +192,8 @@ public static boolean downloadUpdatedBlob(Map conf, BlobStore bl out.close(); } isSuccess = true; +} catch(FileNotFoundException fnf) { --- End diff -- I'd rather leave it as it is, unless we feel confident that it is OK to swallow here. ---
[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user danny0405 commented on a diff in the pull request: https://github.com/apache/storm/pull/2773#discussion_r204992962 --- Diff: storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java --- @@ -197,7 +198,7 @@ public void run() { throw new RuntimeException(e); } } -}, 0, ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS))); +}, 0, ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS))*1000); --- End diff -- Nice catch, `ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS)) * 1000` only the missing whitespace. ---
[GitHub] storm pull request #2773: Blobstore sync bug fix
GitHub user jiangzhileaf opened a pull request: https://github.com/apache/storm/pull/2773 Blobstore sync bug fix 1.Bug fix for blob sync frequency with time unit error. 2.Bug fix for blob sync delete file, add catch NoSuchFileException. 3.Bug fix for blob sync update blob flie, add catch FileNotFoundException You can merge this pull request into a Git repository by running: $ git pull https://github.com/jiangzhileaf/storm blobstore-sync-bug-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2773.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 #2773 commit b2d7f31de318109e708eda060d664acb184b31e7 Author: jiangzhileaf Date: 2018-07-19T08:37:22Z Bug fix for blobstore sync. commit 2b75ade3a6789e755945bbdb89c3ee708b5f30ea Author: jiangzhileaf Date: 2018-07-24T03:18:31Z Specially handle for FileNotFoundException, because it take time for leader nimbus to copy the jar to blobstore commit 849c74198774e95f7aaa1b89a96a8e28012d6119 Author: jiangzhileaf Date: 2018-07-24T07:45:46Z fix blob sync frequency. ---