lee/heritrix3
1
0
Fork 0
mirror of https://github.com/internetarchive/heritrix3.git synced 2026-09-24 06:36:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

do not drop any CrawlURI.data between processing

Browse Source 
Without this change (or other measures), we sometimes get nulls in the
ExtractorYoutubeDL log for containing page information. We'll run this
on QA for a while and see if it causes any problems.

nlevitt [1:59 PM]
https://github.com/internetarchive/heritrix3/blob/master/modules/src/main/java/org/archive/modules/CrawlURI.java#L878
drops some stuff from `CrawlURI.data` after processing a uri, even if it needs to be processed again
there is a list of keys that shouldn’t be dropped (`persistentKeys`), but it is final and private
so if you’re writing your own heritrix module and you want to keep some information in CrawlURI.data, it usually works, except when the url is processed more than once (like when it needs a prereq like robots.txt the first time)
in practice it seems that most data is persisted, that is, most commonly used keys are in `persistentKeys`
in a crawl with pretty standard configuration i’m mostly seeing `prerequisite-uri` dropped and occasionally `fetch-completed-time` and `fetch-began-time` being dropped
i’m highly skeptical of the value of dropping keys at all and i’m tempted to get rid of this entirely, make all the keys persistent in other words
soliciting feedback (edited)

anjackson [2:39 PM]
My immediate reaction is HARD AGREE. It looks like Really Old Code though (https://github.com/internetarchive/heritrix3/blame/7d3eff5269142c77fa4b988396153f4c29d16caa/modules/src/main/java/org/archive/modules/CrawlURI.java#L878)
so the reasons for doing so may have been lost in time.
Hm, looking at usage: https://github.com/internetarchive/heritrix3/blob/a60b2ef3875ad47f57b0c6b3c0b19f86c40a12f7/engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java#L954-L955
engine/src/main/java/org/archive/crawler/frontier/WorkQueueFrontier.java:954-955

                curi.processingCleanup(); // lose state that shouldn't burden
                                          // retry

I guess there's a concern that there may be state in there that is set during a fetch and may cause problems if the same CrawlURI is deferred?
But I'm not aware of anything in the fetch chain that behaves like that.

nlevitt [3:02 PM]
oh, i missed `CrawlURI.addDataPersistentMember(String)` et al. still...
This commit is contained in:
Noah Levitt
2019-04-30 16:20:53 -07:00
parent b7de2bd2ae
commit 37fb6f6b7b
4 changed files with 3 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/src/main/java/org/archive/modules/CrawlURI.java
+1 -68
View File
@@ -31,9 +31,6 @@ import static org.archive.modules.CoreAttributeConstants.A_HTTP_RESPONSE_HEADERS
import static org.archive.modules.CoreAttributeConstants.A_NONFATAL_ERRORS;
import static org.archive.modules.CoreAttributeConstants.A_PREREQUISITE_URI;
import static org.archive.modules.CoreAttributeConstants.A_SOURCE_TAG;
import static org.archive.modules.CoreAttributeConstants.A_SUBMIT_DATA;
import static org.archive.modules.CoreAttributeConstants.A_SUBMIT_ENCTYPE;
import static org.archive.modules.CoreAttributeConstants.A_WARC_RESPONSE_HEADERS;
import static org.archive.modules.SchedulingConstants.NORMAL;
import static org.archive.modules.fetcher.FetchStatusCodes.S_BLOCKED_BY_CUSTOM_PROCESSOR;
import static org.archive.modules.fetcher.FetchStatusCodes.S_BLOCKED_BY_USER;
@@ -76,7 +73,6 @@ import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -248,15 +244,6 @@ implements Reporter, Serializable, OverlayContext, Comparable<CrawlURI> {
* buggy
*/
protected long ordinal;
/**
* Array to hold keys of data members that persist across URI processings.
* Any key mentioned in this list will not be cleared out at the end
* of a pass down the processing chain.
*/
private static final Collection<String> persistentKeys
= new CopyOnWriteArrayList<String>(
new String [] {A_CREDENTIALS_KEY, A_HTTP_AUTH_CHALLENGES, A_SUBMIT_DATA, A_WARC_RESPONSE_HEADERS, A_ANNOTATIONS, A_SUBMIT_ENCTYPE});
/** maximum length for pathFromSeed/hopsPath; longer truncated with leading counter **/
private static final int MAX_HOPS_DISPLAYED = 50;
@@ -875,8 +862,6 @@ implements Reporter, Serializable, OverlayContext, Comparable<CrawlURI> {
this.contentLength = UNCALCULATED;
// Clear 'links extracted' flag.
this.linkExtractorFinished = false;
// Clean the data map of all but registered permanent members.
this.data = getPersistentDataMap();
extraInfo = null;
outLinks = null;
@@ -886,23 +871,6 @@ implements Reporter, Serializable, OverlayContext, Comparable<CrawlURI> {
// XXX er uh surprised this wasn't here before?
fetchType = FetchType.UNKNOWN;
}
public Map<String,Object> getPersistentDataMap() {
if (data == null) {
return null;
}
Map<String,Object> result = new HashMap<String,Object>(getData());
Set<String> retain = new HashSet<String>(persistentKeys);
if (containsDataKey(A_HERITABLE_KEYS)) {
@SuppressWarnings("unchecked")
HashSet<String> heritable = (HashSet<String>)getData().get(A_HERITABLE_KEYS);
retain.addAll(heritable);
}
result.keySet().retainAll(retain);
return result;
}
/**
* @return Credential avatars. Null if none set.
@@ -1132,39 +1100,6 @@ implements Reporter, Serializable, OverlayContext, Comparable<CrawlURI> {
}
return (UURI)getData().get(A_HTML_BASE);
}
public static Collection<String> getPersistentDataKeys() {
return persistentKeys;
}
/**
* Add the key of items you want to persist across
* processings.
* @param s Key to add.
*/
public void addPersistentDataMapKey(String s) {
if (!persistentKeys.contains(s)) {
addDataPersistentMember(s);
}
}
/**
* Add the key of data map items you want to persist across
* processings.
* @param key Key to add.
*/
public static void addDataPersistentMember(String key) {
persistentKeys.add(key);
}
/**
* Remove the key from those data map members persisted.
* @param key Key to remove.
* @return True if list contained the element.
*/
public static boolean removeDataPersistentMember(String key) {
return persistentKeys.remove(key);
}
private void writeObject(ObjectOutputStream stream) throws IOException {
stream.defaultWriteObject();
@@ -1782,9 +1717,7 @@ implements Reporter, Serializable, OverlayContext, Comparable<CrawlURI> {
return containsDataKey(A_FORCE_RETIRE)
&& (Boolean)getData().get(A_FORCE_RETIRE);
}
protected JSONObject extraInfo;
public JSONObject getExtraInfo() {
modules/src/main/java/org/archive/modules/recrawl/FetchHistoryProcessor.java
-1
View File
@@ -67,7 +67,6 @@ public class FetchHistoryProcessor extends Processor {
@Override
protected void innerProcess(CrawlURI puri) throws InterruptedException {
CrawlURI curi = (CrawlURI) puri;
curi.addPersistentDataMapKey(A_FETCH_HISTORY);
HashMap<String, Object> latestFetch = new HashMap<String, Object>();
// save status
modules/src/main/java/org/archive/modules/recrawl/PersistLogProcessor.java
+1 -1
View File
@@ -96,7 +96,7 @@ implements Checkpointable, Lifecycle {
protected void innerProcess(CrawlURI curi) {
log.writeLine(persistKeyFor(curi), " ",
new String(Base64.encodeBase64(
SerializationUtils.serialize((Serializable)curi.getPersistentDataMap()))));
SerializationUtils.serialize((Serializable)curi.getData()))));
}
public void startCheckpoint(Checkpoint checkpointInProgress) {}
modules/src/main/java/org/archive/modules/recrawl/PersistStoreProcessor.java
+1 -1
View File
@@ -40,7 +40,7 @@ public class PersistStoreProcessor extends PersistOnlineProcessor
@Override
protected void innerProcess(CrawlURI curi) throws InterruptedException {
store.put(persistKeyFor(curi),curi.getPersistentDataMap());
store.put(persistKeyFor(curi), curi.getData());
}
@Override