[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-07 Thread Hadoop QA (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16156601#comment-16156601
 ] 

Hadoop QA commented on ZOOKEEPER-2572:
--

+1 overall.  GitHub Pull Request  Build
  

+1 @author.  The patch does not contain any @author tags.

+0 tests included.  The patch appears to be a documentation patch that 
doesn't require tests.

+1 javadoc.  The javadoc tool did not generate any warning messages.

+1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

+1 findbugs.  The patch does not introduce any new Findbugs (version 3.0.1) 
warnings.

+1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

+1 core tests.  The patch passed core unit tests.

+1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1002//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1002//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1002//console

This message is automatically generated.

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-07 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16156575#comment-16156575
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user bitgaoshu commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r137459795
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -409,13 +409,15 @@ public void truncate(long zxid) throws IOException {
 try {
 Files.delete(itr.logFile.toPath());
 } catch (NoSuchFileException e) {
+LOG.info("An NoSuchFileException was thrown when 
delete file {}" +
+", but will continue. Assume this file has been 
deleted successfully.", itr.logFile);
 }
 }
 } finally {
-close(itr);
 if (raf != null) {
 raf.close();
--- End diff --

update


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-07 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16156576#comment-16156576
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user bitgaoshu commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r137459820
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -399,18 +403,20 @@ public boolean truncate(long zxid) throws IOException 
{
 }
 long pos = input.getPosition();
 // now, truncate at the current position
-RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
+raf = new RandomAccessFile(itr.logFile, "rw");
 raf.setLength(pos);
-raf.close();
 while(itr.goToNextLog()) {
-if (!itr.logFile.delete()) {
-LOG.warn("Unable to truncate {}", itr.logFile);
+try {
+Files.delete(itr.logFile.toPath());
+} catch (NoSuchFileException e) {
 }
 }
--- End diff --

update


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-05 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16153763#comment-16153763
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user maoling commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r137008765
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -399,18 +403,20 @@ public boolean truncate(long zxid) throws IOException 
{
 }
 long pos = input.getPosition();
 // now, truncate at the current position
-RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
+raf = new RandomAccessFile(itr.logFile, "rw");
 raf.setLength(pos);
-raf.close();
 while(itr.goToNextLog()) {
-if (!itr.logFile.delete()) {
-LOG.warn("Unable to truncate {}", itr.logFile);
+try {
+Files.delete(itr.logFile.toPath());
+} catch (NoSuchFileException e) {
 }
 }
--- End diff --

- A option:  remain the origin code in the master branch unchanged.just log 
the `itr.logFile` and `zxid`
- B option:  choose `Files.delete`  because it throws `exception` rather 
than `itr.logFile.delete()` returns `boolean` ,**dont't catch it** and it will 
be caught in `Learner.java#syncWithLeader` and log it with `zxid`
- am I right?


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-05 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16153734#comment-16153734
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user maoling commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r137003935
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -409,13 +409,15 @@ public void truncate(long zxid) throws IOException {
 try {
 Files.delete(itr.logFile.toPath());
 } catch (NoSuchFileException e) {
+LOG.info("An NoSuchFileException was thrown when 
delete file {}" +
+", but will continue. Assume this file has been 
deleted successfully.", itr.logFile);
 }
 }
 } finally {
-close(itr);
 if (raf != null) {
 raf.close();
--- End diff --

IMHO, `raf.close();`  should be surrounded with `try-catch` ?


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-04 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152567#comment-16152567
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user bitgaoshu commented on the issue:

https://github.com/apache/zookeeper/pull/356
  
Test Again


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread Hadoop QA (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152091#comment-16152091
 ] 

Hadoop QA commented on ZOOKEEPER-2572:
--

-1 overall.  GitHub Pull Request  Build
  

+1 @author.  The patch does not contain any @author tags.

+0 tests included.  The patch appears to be a documentation patch that 
doesn't require tests.

+1 javadoc.  The javadoc tool did not generate any warning messages.

+1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

+1 findbugs.  The patch does not introduce any new Findbugs (version 3.0.1) 
warnings.

+1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

-1 core tests.  The patch failed core unit tests.

+1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/991//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/991//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/991//console

This message is automatically generated.

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152083#comment-16152083
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user bitgaoshu commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r136739423
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -399,18 +403,20 @@ public boolean truncate(long zxid) throws IOException 
{
 }
 long pos = input.getPosition();
 // now, truncate at the current position
-RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
+raf = new RandomAccessFile(itr.logFile, "rw");
 raf.setLength(pos);
-raf.close();
 while(itr.goToNextLog()) {
-if (!itr.logFile.delete()) {
-LOG.warn("Unable to truncate {}", itr.logFile);
+try {
+Files.delete(itr.logFile.toPath());
+} catch (NoSuchFileException e) {
 }
 }
--- End diff --

- i think ```Files.delete```  can provide more detailed messages if delete 
operation failed.  I'm not sure this change is necessary or not.
- update


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152082#comment-16152082
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user bitgaoshu commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r136739382
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java ---
@@ -395,11 +395,12 @@ else if (qp.getType() == Leader.SNAP) {
 //we need to truncate the log to the lastzxid of the leader
 LOG.warn("Truncating log to get in sync with the leader 0x"
 + Long.toHexString(qp.getZxid()));
-boolean 
truncated=zk.getZKDatabase().truncateLog(qp.getZxid());
-if (!truncated) {
+try {
+zk.getZKDatabase().truncateLog(qp.getZxid());
+} catch (IOException e) {
 // not able to truncate the log
-LOG.error("Not able to truncate the log "
-+ Long.toHexString(qp.getZxid()));
+LOG.error("Not able to truncate the log {}, Unexpected 
exception "
++ Long.toHexString(qp.getZxid()), e);
 System.exit(13);
--- End diff --

update


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152061#comment-16152061
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user bitgaoshu commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r136737378
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -399,18 +403,20 @@ public boolean truncate(long zxid) throws IOException 
{
 }
 long pos = input.getPosition();
 // now, truncate at the current position
-RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
+raf = new RandomAccessFile(itr.logFile, "rw");
 raf.setLength(pos);
-raf.close();
 while(itr.goToNextLog()) {
-if (!itr.logFile.delete()) {
-LOG.warn("Unable to truncate {}", itr.logFile);
+try {
+Files.delete(itr.logFile.toPath());
+} catch (NoSuchFileException e) {
 }
 }
 } finally {
 close(itr);
+if (raf != null) {
+raf.close();
+}
 }
-return true;
 }
--- End diff --

o, sorry. I can't realize what the difference both.  Can you make me clear 
about that, then  I can fix it better?  Thanks


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152030#comment-16152030
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user maoling commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r136732115
  
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/Learner.java ---
@@ -395,11 +395,12 @@ else if (qp.getType() == Leader.SNAP) {
 //we need to truncate the log to the lastzxid of the leader
 LOG.warn("Truncating log to get in sync with the leader 0x"
 + Long.toHexString(qp.getZxid()));
-boolean 
truncated=zk.getZKDatabase().truncateLog(qp.getZxid());
-if (!truncated) {
+try {
+zk.getZKDatabase().truncateLog(qp.getZxid());
+} catch (IOException e) {
 // not able to truncate the log
-LOG.error("Not able to truncate the log "
-+ Long.toHexString(qp.getZxid()));
+LOG.error("Not able to truncate the log {}, Unexpected 
exception "
++ Long.toHexString(qp.getZxid()), e);
 System.exit(13);
--- End diff --

this format of log is right?


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152029#comment-16152029
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user maoling commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r136732072
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -399,18 +403,20 @@ public boolean truncate(long zxid) throws IOException 
{
 }
 long pos = input.getPosition();
 // now, truncate at the current position
-RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
+raf = new RandomAccessFile(itr.logFile, "rw");
 raf.setLength(pos);
-raf.close();
 while(itr.goToNextLog()) {
-if (!itr.logFile.delete()) {
-LOG.warn("Unable to truncate {}", itr.logFile);
+try {
+Files.delete(itr.logFile.toPath());
+} catch (NoSuchFileException e) {
 }
 }
--- End diff --

- Do we have any intentions for changing `itr.logFile.delete() `  to ` 
Files.delete(itr.logFile.toPath())` ? confused about this:
   > Files.delete can throw an exception, which this is useful for error 
reporting and to diagnosed why a file cannot be deleted.
- catch `NoSuchFileException` with even no log ?


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-03 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16152028#comment-16152028
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

Github user maoling commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/356#discussion_r136731827
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -399,18 +403,20 @@ public boolean truncate(long zxid) throws IOException 
{
 }
 long pos = input.getPosition();
 // now, truncate at the current position
-RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
+raf = new RandomAccessFile(itr.logFile, "rw");
 raf.setLength(pos);
-raf.close();
 while(itr.goToNextLog()) {
-if (!itr.logFile.delete()) {
-LOG.warn("Unable to truncate {}", itr.logFile);
+try {
+Files.delete(itr.logFile.toPath());
+} catch (NoSuchFileException e) {
 }
 }
 } finally {
 close(itr);
+if (raf != null) {
+raf.close();
+}
 }
-return true;
 }
--- End diff --

IMHO.should we put code block `Line416-Line418 ` before ` Line415` ?


> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-01 Thread Hadoop QA (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16150449#comment-16150449
 ] 

Hadoop QA commented on ZOOKEEPER-2572:
--

+1 overall.  GitHub Pull Request  Build
  

+1 @author.  The patch does not contain any @author tags.

+0 tests included.  The patch appears to be a documentation patch that 
doesn't require tests.

+1 javadoc.  The javadoc tool did not generate any warning messages.

+1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

+1 findbugs.  The patch does not introduce any new Findbugs (version 3.0.1) 
warnings.

+1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

+1 core tests.  The patch passed core unit tests.

+1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/988//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/988//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/988//console

This message is automatically generated.

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
>Assignee: gaoshu
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-01 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16150427#comment-16150427
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2572:
---

GitHub user bitgaoshu opened a pull request:

https://github.com/apache/zookeeper/pull/356

ZOOKEEPER-2572: Fix potential resource leak in FileTxnLog.truncate



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

$ git pull https://github.com/bitgaoshu/zookeeper fix/ZOOKEEPER-2572

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

https://github.com/apache/zookeeper/pull/356.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 #356


commit 4c21eb8bc56ed4d54dac8d1a16f054f08f2efc2e
Author: bitgaoshu 
Date:   2017-08-31T12:24:40Z

ZOOKEEPER-2572: Fix potential resource leak in FileTxnLog.truncate




> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-09-01 Thread gaoshu (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16150402#comment-16150402
 ] 

gaoshu commented on ZOOKEEPER-2572:
---

In addition, I think the {{itr.logFile.delete() }} can be replaced by 
{{Files.delete(itr.logFile.toPath());}}. If failed, Files.delete can throw an 
exception, which this is useful for error reporting and to diagnosed why a file 
cannot be deleted. Then the function will return void if successful, or throw 
an exception.  Then we can log the detailed error and rethrow it.

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2017-08-30 Thread gaoshu (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16148386#comment-16148386
 ] 

gaoshu commented on ZOOKEEPER-2572:
---

IMO, the exception happened in FileTxnLog.truncate can be got caught in that 
function.   log exception, release resource and return false. Then the dead 
code will be work. 

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2016-09-13 Thread Michael Han (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1546#comment-1546
 ] 

Michael Han commented on ZOOKEEPER-2572:


bq. Does it make sense what I am saying?
I think that is dead code, good catch.

bq. what's the right way of dealing with being unable to truncate the log file?
Currently if a file failed to truncate, the exception will bubble up and got 
caught in 
[Follower.java|https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/Follower.java#L93],
 so the error is handled - but we lose the original (IMO) better error message 
logging of {{LOG.error("Not able to truncate the log "+ 
Long.toHexString(qp.getZxid()));}}. Maybe we can catch the exception at 
syncWithLeader, log the detailed error with zxid, then rethrow it. 

And it certainly sounds odd that we have a boolean return signature which only 
return true...

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
> Fix For: 3.4.10, 3.5.3
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2016-09-13 Thread Edward Ribeiro (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15488628#comment-15488628
 ] 

Edward Ribeiro commented on ZOOKEEPER-2572:
---

My central question is: besides the possible resource leak that Michael pointed 
out, what's the right way of dealing with being unable to truncate the log 
file? Return false if unable to truncate the file in the while loop? To catch 
the exception in {{raf.setLength(pos);}} and set a flag to false? Let any error 
bubble up and remove the {{boolean truncated = }} from the calling methods?

> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
> Fix For: 3.4.10, 3.5.3
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (ZOOKEEPER-2572) Potential resource leak in FileTxnLog.truncate

2016-09-13 Thread Edward Ribeiro (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15488593#comment-15488593
 ] 

Edward Ribeiro commented on ZOOKEEPER-2572:
---

Hey [~hanm], really good catch! :D 

/cc [~fpj], [~breed], [~rakesh_r]

While studying this piece of code, I have seen a curious fact: *this method 
never returns false*. It's either IOException or {{true}}.

But then it's call hierarchy eventually calls {{ZkDatabase.truncateLog}} as 
below (my comments are in upper case below):
{code}
public boolean truncateLog(long zxid) throws IOException {
clear();

// truncate the log
boolean truncated = snapLog.truncateLog(zxid);

// IT WILL NEVER ENTER IN THIS IF
if (!truncated) {
return false;
}

loadDataBase();
return true;
}
{code}

And the method above is called by {{Learner.syncWithLeader}}, again with a 
snippet below that will never be called, plus getting out of the 
{{syncWithLeader}} middway. 

{quote}
boolean truncated=zk.getZKDatabase().truncateLog(qp.getZxid());
// AGAIN, NEVER CAN WILL BE TRUE
if (!truncated) {
// not able to truncate the log
LOG.error("Not able to truncate the log "
+ Long.toHexString(qp.getZxid()));
System.exit(13);
}
{quote}

Does it make sense what I am saying?




> Potential resource leak in FileTxnLog.truncate
> --
>
> Key: ZOOKEEPER-2572
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2572
> Project: ZooKeeper
>  Issue Type: Bug
>  Components: server
>Affects Versions: 3.4.9, 3.5.2
>Reporter: Michael Han
> Fix For: 3.4.10, 3.5.3
>
>
> In FileTxnLog.truncate, we have:
> {code}
> public boolean truncate(long zxid) throws IOException {
> FileTxnIterator itr = null;
> try {
> itr = new FileTxnIterator(this.logDir, zxid);
> PositionInputStream input = itr.inputStream;
> if(input == null) {
> throw new IOException("No log files found to truncate! This 
> could " +
> "happen if you still have snapshots from an old setup 
> or " +
> "log files were deleted accidentally or dataLogDir 
> was changed in zoo.cfg.");
> }
> long pos = input.getPosition();
> // now, truncate at the current position
> RandomAccessFile raf=new RandomAccessFile(itr.logFile,"rw");
> raf.setLength(pos);
> raf.close();
> while(itr.goToNextLog()) {
> if (!itr.logFile.delete()) {
> LOG.warn("Unable to truncate {}", itr.logFile);
> }
> }
> } finally {
> close(itr);
> }
> return true;
> }
> {code}
> {{raf}} here can be potentially in a state of not closed after leaving the 
> method, if there is an (IO) exception thrown from setLength.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)