automatic historycache entry removal corrupts incremental history indexing

[vladak]

The changes for #767 in cset d3dbeac badly interact with incrementally adding history cache entries. I only noticed in indexer logs that each changed file triggers call to both removeFile() and addFile() in IndexDatabase. After said changes, removeFile() deletes history cache for given file.

Thus, the indexing now works like this for a changed file:

1. entries are added to history cache incrementally
2. indexDown() calls removeFile() that removes the history cache for the file
3. addFile() creates the history cache for the file from scratch in populateDocument()

Obviously, this increases indexing time dramatically for repositories that support per-directory history.

[vladak]

Relevant code in indexDown() looks like this:

871          for (File file : files) {

...

890                      if (uidIter != null) {
891                          String uid = Util.path2uid(path,
892                              DateTools.timeToString(file.lastModified(),
893                              DateTools.Resolution.MILLISECOND)); // construct uid for doc
894                          BytesRef buid = new BytesRef(uid);
895                          while (uidIter != null && uidIter.term() != null
896                                  && uidIter.term().compareTo(emptyBR) !=0
897                                  && uidIter.term().compareTo(buid) < 0) {
898                              removeFile();
899                              BytesRef next = uidIter.next();
900                              if (next==null) {uidIter=null;}
901                          }
902  
903                          if (uidIter != null && uidIter.term() != null
904                                  && uidIter.term().bytesEquals(buid)) {
905                              BytesRef next = uidIter.next(); // keep matching docs
906                              if (next==null) {uidIter=null;}
907                              continue;
908                          }
909                      }
910                      try {
911                          addFile(file, path);
912                      } catch (Exception e) {
913                          LOGGER.log(Level.WARNING,
914                                  "Failed to add file " + file.getAbsolutePath(),
915                                  e);
916                      }

Obviously, changed file is first removed (from the index, xref, history cache) and then re-added.

I believe that the cycle on lines 895-897 makes sure that directory traversal is in sync with term traversal. E.g. if I have a repository with files A.txt and B.txt and A.txt is removed, indexDown() will cycle through the list of files in given directory and will hit file B.txt first (because there is no A.txt file anymore), in this cycle it will perform the "uid" check for A.txt first because it comes first in the ordering (that is based on file paths with appended time value as visible in path2uid()), removes its data and then will continue to the term corresponding to B.txt, removes its data but then it will go to line 911 to re-add its data again.

There is also call to removeFile() in IndexDatabase#update() after indexDown() returns which I believe clears data for the trailing files.

Hopefully there is a way how to distinguish the case of data refresh and file removal.

[vladak]

Curiously, IndexChangedListener.java contains fileUpdate() however that is not used anywhere.