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

1.0 release status #62

Closed
twmb opened this issue Aug 17, 2021 · 6 comments
Closed

1.0 release status #62

twmb opened this issue Aug 17, 2021 · 6 comments

Comments

@twmb
Copy link
Owner

twmb commented Aug 17, 2021

1.0 is coming soon. This issue tracks what I'd like to accomplish beforehand.

  • Possible record loss on unclean kill of consumer when using automatic offset commit #57 should be addressed
  • RetryTimeout => rename RetryTimeoutFn, add new RetryTimeout option with a static time.Duration argument
  • RetryBackoff => rename RetryBackoffFn (a static RetryBackoff will not be added, because linear static backoff is not a great backoff strategy)
  • ProduceRetries => rename RecordRetries
  • add func DialTLSConfig(*tls.Config) option, which will trigger dialing by tls (simplifies setting up tls)

Some of these are breaking changes to things that are currently seldom used.

twmb added a commit that referenced this issue Aug 17, 2021
For #62.

Using RetryTimeout is awkward, and the usual use case is to just have a
static timeout. This splits the current function based approach into
RetryTimeoutFn, and changes RetryTimeout into a static time.Duration
based option.
@twmb
Copy link
Owner Author

twmb commented Aug 17, 2021

v0.10.0 addresses all but #57.

A few ideas were included in the original comment but have since been nixed due to not really being any better than what exists.

@twmb
Copy link
Owner Author

twmb commented Aug 25, 2021

v0.10.2 addresses #57. This release will sit for a short amount of time, if no bug reports come in, v1.0.0 will be released.

@twmb
Copy link
Owner Author

twmb commented Aug 25, 2021

Another 1.0 rename that does not need a dedicated tag before 1.0:
AllowedConcurrentFetches => MaxConcurrentFetches

@twmb
Copy link
Owner Author

twmb commented Aug 31, 2021

I've written down the entire API, proposed changes for a batch of options, and bounced the proposals back and forth a bit. I think the following set of options should be changed as well before a 1.0 release:

ConnTimeoutOverhead => RequestTimeoutOverhead
  - Why? Request is clearer as to the intent of what this timeout is
Balancers => BalanceStrategies
  - Why? This is closer to the Kafka terminology, and more similar to what
    Sarama uses. I also like that strategies implies these are just algorithms.

CommitCallback => AutoCommitCallback
  - Why? This function is only used for autocommitting. Manual committing
    always requires explicit callbacks already.

OnAssigned => OnPartitionsAssigned
OnRevoked => OnPartitionsRevoked
OnLost => OnPartitionsLost
  - Why? This better implies the meaning of the function.

BatchCompression => ProducerBatchCompression
BatchMaxBytes => ProducerBatchMaxBytes
Linger => ProducerLinger
  - Why? It is conceivable that BatchCompression / BatchMaxBytes could be
    confused with consumer fetch behavior. Adding a Producer prefix makes these
    options unambiguously clearer what they are for.

StopOnDataLoss => StopProducerOnDataLossDetected
OnDataLoss => ProducerOnDataLossDetected
  - These are already niche options, and I want them to be even more explicit
    because they are a bit confusing.

RecordTimeout => RecordDeliveryTimeout
  - This is closer to the Kafka terminology (delivery.timeout.ms), while
    retaining that this is specific per record (internally, per batch).

The following were close, but I don't think make the cut:

RequestRetries => MaxRequestRetries
  - Why not? Retries is already a count, Max does not add that much clarity.
RetryTimeout => RequestRetryTimeout
RetryTimeoutFn => RequestRetryTimeoutFn
  - Why not? Request prefix just starts to get unnecessarily descriptive.
    RequestRetryTimeout may be slightly clearer, but not compellingly so.
RequiredAcks => ProducerRequiredAcks
  - Why not? Only producing requires acks, so RequiredAcks is unambiguous
    (similar to RecordRetries, etc).

Everything else was less compelling.

I plan to push these as v0.11.0 within the day, and wait another day for any dissenting feedback before I tag v1.0. Also, #73 is easy and can go with v0.11.0.

@twmb twmb mentioned this issue Aug 31, 2021
@twmb
Copy link
Owner Author

twmb commented Sep 1, 2021

All of the above, minus Balancers, was implemented in v0.11.0. This serves as rc2. Barring any further suggestions, 1.0 is imminent.

@twmb
Copy link
Owner Author

twmb commented Sep 3, 2021

This has been released!

@twmb twmb closed this as completed Sep 3, 2021
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

No branches or pull requests

1 participant