apache / apache/lucene

ExternalRefSorter should use OfflineSorter's actual writer for writing the input file [LUCENE-7477]

Open
#8,529 3 comments 0 reactions 1 assignee Claimed by @dweiss View on GitHub
legacy-jira-priority:Minor type:enhancement
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

Consider this constructor in ExternalRefSorter:

```Java
public ExternalRefSorter(OfflineSorter sorter) throws IOException {
this.sorter = sorter;
this.input = sorter.getDirectory().createTempOutput(sorter.getTempFileNamePrefix(), "RefSorterRaw", IOContext.DEFAULT);
this.writer = new OfflineSorter.ByteSequencesWriter(this.input);
}
```

The problem with it is that the writer for the initial input file is written with the default `OfflineSorter.ByteSequencesWriter`, but the instance of `OfflineSorter` may be unable to read it if it overrides `getReader` to use something else than the default.

While this works now, it should be cleaned up (I think). It'd be probably ideal to allow `OfflineSorter` to generate its own temporary file and just return the ByteSequencesWriter it chooses to use, so the above snippet would read:

```Java
public ExternalRefSorter(OfflineSorter sorter) throws IOException {
this.sorter = sorter;
this.writer = sorter.newUnsortedPartition();
}
```

This could be also extended so that `OfflineSorter` is in charge of managing its own (sorted and unsorted) partitions. Then `sort(String file)` would simply become `ByteSequenceIterator sort()` (or even `Stream sort()` as Stream is conveniently `AutoCloseable`). If we made `OfflineSorter` implement `Closeable` it could also take care of cleaning up any resources it opens in the directory we pass to it. An additional bonus would be the ability to dodge the final internal merge(1) – if we manage sorted and unsorted partitions then there are open possibilities of returning an iterator that dynamically merges from multiple partitions.

---
Migrated from [LUCENE-7477](https://issues.apache.org/jira/browse/LUCENE-7477) by Dawid Weiss (@dweiss), updated Sep 29 2020
Attachments: [LUCENE-7477.patch](https://apache.github.io/lucene-jira-archive/attachments/LUCENE-7477/LUCENE-7477.patch)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.