Re: [Nutch-dev] code to make parsing optional in fetcher

On Tue, Oct 19, 2004 at 11:52:36AM +0200, Andrzej Bialecki wrote:
> 
> Please see my comments below. A general comment on the logic in 
> Fetcher.java - even though the command-line option is called noParsing 
> perhaps the logic would look a bit nicer if the sense of this option was 
> reversed, i.e. "boolean parse = true" was used by default.

I was in fact debating with myself on this. Will modify.

> 
> 
> >+      // If noParsing is set true, ./fetcher is renamed to 
> >./fetcher_output.
> >+      // Next, in parsing phase (using ParseSegment.java),
> >+      // ./fetcher_output will be consulted and new ./fetcher created
> >+      // with fetcher status NOT_FOUND properly set for any
> >+      // ParseException thrown.
> 
> Please document this somewhere in the externally visible javadoc and 
> opionally Wiki - this could be surprising, especially if you 
> accidentally end up with a mix of segments pre-parsed and not pre-parsed...

Yes, a more verbose note is due.

> 
> >+      if (noParsing)
> >+        nfs.rename(new File(directory, FetcherOutput.DIR_NAME),
> >+          new File(directory, FetcherOutput.DIR_NAME+"_output"));
> 
> I think it would be good to put it as another constant in FetcherOutput, 
> so that other tools can check for the existence of this directory 
> without using literal names.

This is another place where I was not so sure,
since i did not want to stray away from current fetcher.
Now that you suggest it, I have no objection.

> 
> >+++ nutch-cvs-20041018.xing/src/java/net/nutch/tools/ParseSegment.java 
> >2004-10-18 23:42:08.000000000 -0700
> 
> >+  /** Display the status of the parser run. */
> >+  public void status() {
> >+    long ms = System.currentTimeMillis() - start;
> >+    LOG.info("status: "
> >+             + pages + " pages, "
> >+             + errors + " errors, "
> >+             + bytes + " bytes, "
> >+             + ms + " ms");
> >+    LOG.info("status: "
> >+             + (((float)pages)/(ms/1000.0f))+" pages/s, "
> >+             + (((float)bytes*8/1024)/(ms/1000.0f))+" kb/s, "
> >+             + (((float)bytes)/pages) + " bytes/page");
> >+  }
> 
> This comment is not specific to this particular tool, it's rather a 
> general comment on the status reporting from various tools in Nutch - I 
> think that until we grow a full JMX support it makes sense also to 
> export the current status data through API. The reson is that if you 
> want to use this or that tool programmatically (e.g. through calling 
> it's main() method) in a system with different UI than the log files, 
> then currently there is no way to poll the tool for its progress.
> 
> I once created a patch to Fetcher which added Fetcher.Status class 
> (POJO), which you could use to export current fetching status. Now again 
> I have a need to use this, so I may revisit it, and then it would be 
> good if e.g. your tool, or any other tool, could report its status in a 
> similar way. This would be also a good start for the JMX beans 
> interface, as far as I understand them, that is ;-)
> 
> So I propose to add an inner class:
> 
> public static class Status {
>       public String segment; // segment name
>       public String stage; // current stage name, e.g. "sorting"
>       public float pages, bytes;
>       public long start; // start time millis.
>       // possibly per-thread statistics, if you like
>       // ...
> }

Could you resurrect Fetcher.Status, or have something more general?
I will adopt it.

> 
> and a corresponding method:
> 
> public ParseSegment.Status getStatus() {...}
> 
> >+        if (fetcherReader.key() != key.get())
> >+          throw new IOException("Mismatch between entries under "
> >+                + fetcherDir+"_output" + " and in " + sortedFile);
> 
> Hmmm... You have this code in several places. Have you ever actually 
> encountered this problem? In my experience, due to the append-only 
> nature of SequenceFiles, either you get all files in sync up to the 
> first corrupt entry, or you get nothing (e.g. only the "data" file 
> without a corresponding "index"), but I have never so far seen these 
> files going out of sync...

Certainly redundant. Time for clean up.

> 
> >+      // safeguard! make sure there are identical entries
> >+      // in (fetcher, content) and in (parseData, parseText)
> >+      if (count != (pages+errors))
> >+        throw new IOException("Missing entries: expect "+(pages+errors)
> >+          +", but only have "+count+" entries");
> 
> Well, normally the source of problems here is a crashed or interrupted 
> fetcher, which leaves you with corrupt sequence files. However, I don't 
> think this is a critical issue - other tools can handle such conditions, 
> so perhaps only a warning is needed here.
> 
> >+    try {
> >+      parseSegment.sort(clean);
> >+      parseSegment.save(clean);
> 
> What is the reason behind the "clean" flag to save()? It's not currently 
> used anywhere inside save()...

nfs.delete(new File(sortedFile));

John


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl