Replace A_TIMESTAMP with A_FETCH_BEGAN_TIME

[hennekey]

It seems that A_TIMESTAMP went out of favor quite a long time ago.
A_FETCH_BEGAN_TIME is used within FetchHistoryProcessor and throws an
exception as is because of it.

[ato]

This seems reasonable but I'm not familiar enough with this module to confidently review this. I'm hoping someone more knowledgeable about this comments, otherwise I'll merge it in a few days if we don't hear anything.

[nlevitt]

This is @kngenie's code. I don't know if it's still used.

[hennekey]

I was trying to use it in conjunction with the HBase recrawl modules but everything besides these class seems to use A_FETCH_BEGAN_TIME and so they're writing to the wrong attribute and FetchHistoryProcessor throws an exception here:

heritrix3/modules/src/main/java/org/archive/modules/recrawl/FetchHistoryProcessor.java

Line 125 in 9927e13

revisit.setRefersToDate((Long)history[1].get(A_FETCH_BEGAN_TIME));

[anjackson]

We use A_TIMESTAMP but can switch if that makes sense. However, I'm having trouble seeing what exactly you are referring to @hennekey because your links go to your private GitHub Enterprise instance. Any chance you can open it up? Or refer to this repo instead?

[hennekey]

@anjackson Sorry about that, I've updated the link to the code.

Any explanation about how you make use of A_TIMESTAMP would be helpful so I can do the right thing.

[anjackson]

Thanks, but I think your original link refers to:

heritrix3/contrib/src/main/java/org/archive/modules/recrawl/wbm/WbmPersistLoadProcessor.java

Lines 519 to 521 in 0581170

	// A_TIMESTAMP has been used for sorting history long before A_FETCH_BEGAN_TIME
	// field was introduced. Now FetchHistoryProcessor fails if A_FETCH_BEGAN_TIME is
	// not set. We could stop storing A_TIMESTAMP and sort by A_FETCH_BEGAN_TIME.

?

We use it in much the same way, I think. We use it for deduplication and for checking how recently we visited to see if we need to re-crawl yet. But it looks like I hit similar issues because I populate both fields.

I'm not sure how important these semantics are -- the timestamp in question may not necessarily be the time the fetch began, but in practice I think it always is, so I think it's fine to reduce it all to that name.

[hennekey]

Yeah, I did reference that comment.
It occurs to me now that perhaps semantically A_TIMESTAMP is meant to be the time the history map entry was created for that CrawlURI and A_FETCH_BEGAN_TIME is more specifically the point in time that the fetch action started.
I defer to you to the semantics; you've obviously got more experience with it. I'd be happy to make whatever changes we deem necessary as a result.

[hennekey]

Your use of this constant also makes it clear that it's a more dangerous change than I thought. It is public afterall so there may be other users.

Perhaps the proper solution is to clarify the semantics and use the proper value in FetchHistoryProcessor instead?