Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] csw harvester encountering an empty page has to continue harvesting #3713

Closed
wants to merge 1 commit into from
Closed

[WIP] csw harvester encountering an empty page has to continue harvesting #3713

wants to merge 1 commit into from

Conversation

cmangeat
Copy link
Collaborator

condition on next record not changing should suffice avoiding infinite
loop. furthermore, empty page condition induce an random behaviour,
harvesting will stop on a fully empty page, incomplete page are accepted,
this depending on page size and records order.

condition on next record not changing should suffice avoiding infinite
loop. furthermore, empty page condition induce an random behaviour,
harvesting will stop on a fully empty page, incomplete page are accepted,
this depending on page size and records order.
@fxprunayre
Copy link
Member

@etj any objections for this ? it was added in 3d6b9ab.

Indeed it may happen if asking for ISO19139 and a page contains only ISO19110 records. Then loop stops here.

@fxprunayre fxprunayre requested a review from etj May 14, 2019 08:06
@fxprunayre fxprunayre added this to the 3.8.0 milestone May 14, 2019
log.warning("Forcing harvest end since numberOfRecordsReturned = 0");
break;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also change

start = nextRecord;

line 401.

@josegar74 any opinion?

Copy link
Contributor

@juanluisrp juanluisrp Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing something like

if (nextRecord !=null && nextRecord < start) {
        log.warning(String.format("Forcing harvest end since nextRecord < start (nextRecord = %d, start = %d", nextRecord, start));
        break;
}

@cmangeat cmangeat changed the title csw harvester encountering an empty page has to continue harvesting [WIP] csw harvester encountering an empty page has to continue harvesting Jun 20, 2019
@fxprunayre fxprunayre modified the milestones: 3.8.0, 3.8.1 Jun 26, 2019
@fxprunayre fxprunayre modified the milestones: 3.8.1, 3.8.2 Sep 23, 2019
@fxprunayre fxprunayre modified the milestones: 3.8.2, 3.8.3 Oct 29, 2019
@@ -389,13 +389,6 @@ private void searchAndAlign(CswServer server, Search s, Set<String> uuids,
break;
}

// Another way to escape from an infinite loop

if (returnedCount == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check was introduced as a workaround to #1429. Would the harvester still work for a server not returning nextRecord?

juanluisrp added a commit to juanluisrp/core-geonetwork that referenced this pull request Feb 12, 2020
Some server aren't compliant with CSW specification. In some cases they can  return a value
different of 0 for nextRecord when the harvester has reached the end of the available
results.
This commit stops the harvesting loop if the value of `nextResult` returned by the server
is smaller than the current `start` record.

Could be a complement to geonetwork#3713.
@cmangeat cmangeat closed this Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants