Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  closed
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  metrics-help |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Looks good!  Rebased, squashed, merged, pushed, closing!  Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics-help |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Looks fine.

 I added another test and cure for a corrupt history file.
 Please see [https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20521-3=e02132ee0903994ec0702144ebd76cb425fb04cc
 my branch task-20521-3].

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics-help |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * status:  new => needs_review


Comment:

 Please review [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/log/?h=task-20521 my updated task-20521 branch] with another fixup
 commit and the requested tests.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics-help |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by iwakeh):

 * keywords:   => metrics-help


Comment:

 These tests would also be a good start for a volunteer.  Thus, adding the
 keyword.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by karsten):

 Haha, okay.  In that case we're back at needing unit tests for the
 `DescriptorReader`/`DescriptorReaderImpl` before merging my (squashed)
 task-20521 branch.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Replying to [comment:6 karsten]:
 > Hmm, are you sure that the [https://gitweb.torproject.org/user/karsten
 /metrics-
 lib.git/commit/?h=task-20521=ad7ab8afe259a7737d90924b6d46e447144a0a6c
 smaller fixes in my branch] are insufficient for Java 7 compatibility?

 That branch is fine java 7.  Just ignore my comment:5 (I need more coffee
 ;-)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by karsten):

 Hmm, are you sure that the [https://gitweb.torproject.org/user/karsten
 /metrics-
 lib.git/commit/?h=task-20521=ad7ab8afe259a7737d90924b6d46e447144a0a6c
 smaller fixes in my branch] are insufficient for Java 7 compatibility?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Oops, I was in fact relying on the javac-task catching it, after all
 source and target are set to 1.7.
 Please find the changes to java 7
 [https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20521-2=425e1ab3178f60b1c392c50a013f164cb2091f09
 here].
 Will look into that java versioning issue.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * status:  needs_review => new


Comment:

 Replying to [comment:3 iwakeh]:
 > Please find a [https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20521-2=c8707e56904119fc072b700b9f839ac371c1d83e
 commit] on top of your branch with some modernization (reduced number of
 lines) and a checkstyle complaint removed.

 Oops, I totally missed that checkstyle complaint.  But I think your fix
 only removes the complaint and lets Javadoc think that the `@deprecated`
 line is yet another paragraph, which is not what we want.  Indenting the
 two lines below that line is the better fix.  Removed that part from your
 commit and added a fixup commit to remove the checkstyle warning.

 I also found that you're using `Files` methods that are introduced in Java
 8, but we're still at Java 7.  Well, Eclipse found that for me.  I added
 another fixup commit for those.  Can you configure your IDE to use the
 Java 7 SE library?  (But okay, I won't say anything after complaining
 about unresolved checkstyle warnings in other patches and then overlooking
 one myself, heh.)

 Please find [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/log/?h=task-20521 my updated task-20521 branch].

 > Wouldn't this be the right time to add a test class for
 `DescriptorReader`(Impl)?

 H, yes.  Setting this ticket to new and not merging yet until we
 have such a test class.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---

Comment (by iwakeh):

 Please find a [https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20521-2=c8707e56904119fc072b700b9f839ac371c1d83e
 commit] on top of your branch with some modernization (reduced number of
 lines) and a checkstyle complaint removed.

 Wouldn't this be the right time to add a test class for
 `DescriptorReader`(Impl)?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by iwakeh):

 * cc: iwakeh (added)


Comment:

 cc'ed myself in order to have this on my list.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+---
Changes (by karsten):

 * status:  new => needs_review


Comment:

 Please review [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/log/?h=task-20521 my branch task-20521].

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #20521 [Metrics/metrics-lib]: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate methods for loading and saving a history file

[Tor Bug Tracker & Wiki]

#20521: Deprecate `DescriptorReader.setExcludeFiles()` and add two separate 
methods
for loading and saving a history file
-+---
 Reporter:  karsten  |  Owner:  karsten
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:  metrics-lib 1.6.0
Component:  Metrics/metrics-lib  |Version:
 Severity:  Normal   |   Keywords:
Actual Points:   |  Parent ID:
   Points:   |   Reviewer:
  Sponsor:   |
-+---
 The history file implementation in `DescriptorReader` has a design bug for
 much too long that I'd finally want to fix.

 The issue is that `DescriptorReader` writes the history file passed in
 `setExcludeFiles()` immediately after reading and parsing the last
 descriptor and putting it into the queue, regardless of whether the
 application has finished processing those descriptors.

 If the application fails after the history file is written, it may not be
 able to process descriptors in the next execution that have still been in
 the queue at the time of failing.

 One way to reduce that gap would be to have `BlockingIteratorImpl` tell
 `DescriptorReader` when the application has first learned that `hasNext()`
 returned `false` or that `next()` threw a `NoSuchElementException`.  The
 benefit of that solution would be that no interface change would be
 required.  The downside would be that the application might not have fully
 cleaned up at the time of learning that there are no further descriptors
 available.  In one example, the application would close a database import
 file, perform the database import, and only write the history file after
 learning that the database import has succeeded.

 A better solution would be to decouple saving the history file to disk
 from completing the descriptor read operation.  We would replace the
 `setExcludeFiles()` method by a `setHistoryFile()` and a
 `saveHistoryFile()` method.  Applications would use `setHistoryFile()`
 before starting to read descriptors, process all descriptors, perform any
 cleaning up, and then call `saveHistoryFile()`.

 Here's an example of the code that this would save in applications that
 currently work around this known design bug by loading and saving history
 files themselves:

 {{{
 diff --git
 a/modules/hidserv/src/org/torproject/metrics/hidserv/Parser.java
 b/modules/hidserv/src/org/torproject/metrics/hidserv/Parser.java
 index 2ef404e..215b0c9 100644
 --- a/modules/hidserv/src/org/torproject/metrics/hidserv/Parser.java
 +++ b/modules/hidserv/src/org/torproject/metrics/hidserv/Parser.java
 @@ -96,33 +96,7 @@ public class Parser {
public void readParseHistory() {
  if (this.parseHistoryFile.exists()
  && this.parseHistoryFile.isFile()) {
 -  SortedMap excludedFiles =
 -  new TreeMap();
 -  try {
 -BufferedReader br = new BufferedReader(new FileReader(
 -this.parseHistoryFile));
 -String line;
 -while ((line = br.readLine()) != null) {
 -  try {
 -/* Each line is supposed to contain the last-modified time
 and
 - * absolute path of a descriptor file. */
 -String[] parts = line.split(" ", 2);
 -excludedFiles.put(parts[1], Long.parseLong(parts[0]));
 -  } catch (NumberFormatException e) {
 -System.err.printf("Illegal line '%s' in parse history.  "
 -+ "Skipping line.%n", line);
 -  }
 -}
 -br.close();
 -  } catch (IOException e) {
 -System.err.printf("Could not read history file '%s'.  Not "
 -+ "excluding descriptors in this execution.",
 -this.parseHistoryFile.getAbsolutePath());
 -  }
 -
 -  /* Tell the descriptor reader to exclude the files contained in the
 -   * parse history file. */
 -  this.descriptorReader.setExcludedFiles(excludedFiles);
 +  this.descriptorReader.setHistoryFile(this.parseHistoryFile);
  }
}

 @@ -130,33 +104,7 @@ public class Parser {
 * and absolute paths to the parse history file to avoid parsing these
 * files again, unless they change until the next execution. */
public void writeParseHistory() {
 -
 -/* Obtain the list of descriptor files that were either parsed now or
 - * that were skipped in this execution from the descriptor reader. */
 -SortedMap excludedAndParsedFiles =
 -new TreeMap();
 -excludedAndParsedFiles.putAll(
 -this.descriptorReader.getExcludedFiles());
 -
 excludedAndParsedFiles.putAll(this.descriptorReader.getParsedFiles());
 -try {
 -  this.parseHistoryFile.getParentFile().mkdirs();
 -  BufferedWriter bw = new BufferedWriter(new FileWriter(
 -