feat(sentry): Add missing --no-strict-offset-reset and --auto-offset-reset for consumers

[adonskoy]

No description provided.

[patsevanton]

When using Apache Kafka under high load conditions and applying the auto.offset.reset parameter with a value of earliest, several important consequences can occur:

1. Message re-processing:

• If auto.offset.reset is set to earliest, when a new consumer is connected (consumer) or when restoring operation after a failure, the consumer will start reading messages from the beginning of the current partition. This means that any messages that have already been processed will be processed again.

2. ** Increased load on the consumer:**

• Repeated processing of all messages can significantly increase the load on the consumer, especially if there are a large number of messages in a topic.
• If the consumer is not designed to handle this load, it may cause delays in processing new messages, increase resource consumption (CPU, memory), and ultimately degrade system performance.

3. Possible message delivery problems:

• If the consumer cannot handle the re-processing of all messages, it may cause new messages to be processed with a delay or not delivered on time at all.
• In some cases, this can lead to business logic violations, especially if the system expects real-time message processing.

May be set autoOffsetReset in values.yaml

autoOffsetReset: "none"  # latest, earliest, none

?

[adonskoy]

@patsevanton, --auto-offset-reset value is only used when --no-strict-offset-reset is set, and also earliest is the default value

I don't really like this design myself, but that's how it's done in other consumer now, and I wouldn't want to change the way we work with arguments in this PR too much

[patsevanton]

For backwards compatibility, I would make the code like this:

          {{- if .Values.sentry.postProcessForwardErrors.autoOffsetReset }}
          - "--auto-offset-reset"
          - "{{ .Values.sentry.postProcessForwardErrors.autoOffsetReset }}"
          {{- end }}
          {{- if .Values.sentry.postProcessForwardErrors.noStrictOffsetReset }}
          - "--no-strict-offset-reset"
          {{- end }}

and values.yaml

    # autoOffsetReset: "none"  # latest, earliest, none
    # noStrictOffsetReset: false

but it's not for me to decide.

[adonskoy]

@Mokto, WDYT?

[adonskoy]

For backwards compatibility, I would make the code like this:

          {{- if .Values.sentry.postProcessForwardErrors.autoOffsetReset }}
          - "--auto-offset-reset"
          - "{{ .Values.sentry.postProcessForwardErrors.autoOffsetReset }}"
          {{- end }}
          {{- if .Values.sentry.postProcessForwardErrors.noStrictOffsetReset }}
          - "--no-strict-offset-reset"
          {{- end }}

and values.yaml

    # autoOffsetReset: "none"  # latest, earliest, none
    # noStrictOffsetReset: false

but it's not for me to decide.

There are already consumers with that arg (for example)

This is not the best option, but it works. Pod templates require a lot of refactoring and I plan to do this soon

But now my goal is to add the missing functionality to the chart so that the system can function in environments with limited resources

[Mokto]

Yeah I'm fine with adding some feature to the chart but this looks pretty much like a breaking change to me.

I would recommend we comment what's in values.yaml

[adonskoy]

Yeah I'm fine with adding some feature to the chart but this looks pretty much like a breaking change to me.

I would recommend we comment what's in values.yaml

--auto-offset-reset has a default value - the same as what I specified in values. but this argument has no effect unless --no-strict-offset-reset is specified, but it is disabled by default

--auto-offset-reset has no default value. Will fix it soon.

[adonskoy]

Now it's the conditional argument

[Mokto]

Thanks!

[bmassemin]

Error: Invalid value for '--auto-offset-reset': '' is not one of 'error', 'earliest', 'latest'. on suba containers

[patsevanton]

@bmassemin i testing fix...

[adonskoy]

Error: Invalid value for '--auto-offset-reset': '' is not one of 'error', 'earliest', 'latest'. on suba containers

@bmassemin, can you send your values ​​for snuba consumers? Can't reproduce this

[adonskoy]

Oh, I see. There are several consumers in which this argument was added without a condition several years ago, and for them there is now no default value in values

[bmassemin]

Error: Invalid value for '--auto-offset-reset': '' is not one of 'error', 'earliest', 'latest'. on suba containers

@bmassemin, can you send your values ​​for snuba consumers? Can't reproduce this

I don't have specific values except for resources