fix: hard limit on field size while parsing line protocol

[lesam]

Per https://docs.influxdata.com/enterprise_influxdb/v1.9/write_protocols/line_protocol_reference/
we only support 64KB, but 1MB is a more realistic practical limit. Before this commit there was
no enforcement of field value size.

Closes #21841

Describe your proposed changes here.

• CHANGELOG.md updated with a link to the PR (not the Issue)
• Well-formatted commit messages
• Rebased/mergeable
• Tests pass

[lesam]

We don't catch everywhere that can build a point, but this covers the write ingress which is the most important one.

[gwossum]

Implementation and test cases looks good, have test cases for maximum possible field size and 1 greater.

It'd be cool if the test checked the error message in the HTTP body since there's multiple possible errors, but none of the other tests in server_test.go seem to check error messages. I won't hold that against this test.

[lesam]

Closing as we want to make this configurable

[gwossum]

Eliminating Engine.FieldValidator seems reasonable, since I can't find any use other than the default validator. If we get rid of Engine.FieldValidator, should we also get rid of EngineOptions.FieldValidator? There might be confusion in the future if someone thinks they can set that to change the field validation behavior.

[lesam]

This is in EngineOptions - I don't see any FieldValidator remaining in the code?

[gwossum]

I'm not sure why I thought there was an Engine.FieldValidator. I thought there was a Shard.FieldValidator, typed Engine.FieldValidator, but it was really setting the value in EngineOptions.FieldValidator. My mistake, carry on!

[gwossum]

A possible optimization here is to compare pointSize to MaxFieldValueLength before the loop, and set skipSizeValidation = true before starting the loop if pointSize <= MaxFieldValueLength. It's possible the compiler is already performing this loop invariant hoisting, but we can make sure it is. Even if the compiler is already doing the work, this might improve readability. It would look something like:

    pointSize := point.StringSize()
    if pointSize <= MaxFieldValueLength {
        skipSizeValidation = true
    }
    iter := point.FieldIterator()
    for iter.Next() {
        if !skipSizeValidation {
            if iter.Type() == models.String {

[lesam]

I think the code as written is simpler, and I don't want to make it less simple for a single comparison - I doubt that would show up in any profiling.

It is debatable whether I should even be doing the pointSize optimization, but I think it is a clear enough win to not re-allocate a ton of strings during StringValue that it is ok to add the check.

[gwossum]

Looks good to me. Test cases for both sides of boundary condition.