Fix DynamoDB getAllRecords logic when 1MB query limit is reached

[smallinsky]

No description provided.

[greedy52]

Fix looks good. I think an alternative is to keep a running limit like

for i := 0; i < backend.DefaultRangeLimit/100; i++ {
	limitRemaining := 0
	if limit > 0 {
		limitRemaining = limit - len(results.records)
	}
	re, err := b.getRecords(ctx, prependPrefix(startKey), prependPrefix(endKey), limitRemaining, result.lastEvaluatedKey)

...
}

This can make sure we WONT fetch more than what we need. Also maybe no need to do len(result.records) >= limit check i believe.

[fspmarshall]

Fix for the 1mb issue looks good. It seems like we're just mishandling limits here generally tho.

The backend should return at most limit, and should default to using backend.DefaultRangeLimit if no limit is specified. I think we should change getAllRecords to set the limit to backend.DefaultRangeLimit if no limit is specified. Also, instead of passing in the same limit for each subsequent call to getRecords, we should pass in limit - len(result.records), so that we get at most exactly limit worth of items.

If you have the time, can you make those changes here b4 merging?

[smallinsky]

@fspmarshall

we should pass in limit - len(result.records), so that we get at most exactly limit worth of items.

This approach will have main drawback. Namely the limit is a applied before filtering in dynamoDB case so if there are 1000x elements that doesn't match filtering rule and there is only one element that matches it will means tha in case of limit set to 1 the dynamoDB API in worst case will be called 1001x times.

Regarding:

The backend should return at most limit, and should default to using backend.DefaultRangeLimit if no limit is specified

Agree, I will change this.