Skip to content

Comments

[BUG FIX]Kinesis lag keep increasing when there is no more new data for kinesis stream#11006

Merged
suneet-s merged 4 commits intoapache:masterfrom
zhangyue19921010:kinesis-lag-metrics-bug-fix
Mar 19, 2021
Merged

[BUG FIX]Kinesis lag keep increasing when there is no more new data for kinesis stream#11006
suneet-s merged 4 commits intoapache:masterfrom
zhangyue19921010:kinesis-lag-metrics-bug-fix

Conversation

@zhangyue19921010
Copy link
Contributor

@zhangyue19921010 zhangyue19921010 commented Mar 17, 2021

Fixes #11005.

Description

check the value of AFTER_SEQUENCE_NUMBER. If empty it means there is no more new data for current stream. So just return 0L here.


Key changed/added classes in this PR
  • KinesisRecordSupplier.java

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@zhangyue19921010
Copy link
Contributor Author

Hi @clintropolis could you please take a look at this issue at your convince? I find you introduce this useful metrics in #9509. Maybe you will be more familiar with it :)

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

👍 thanks

});
}

private GetRecordsResult getRecords(String iteratorType, String offsetToUse, StreamPartition<String> partition)
Copy link
Member

Choose a reason for hiding this comment

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

nit: might be nice to name this method getRecordsForLag or something similar to indicate its limited purpose (the .withLimit(1) makes it probably not really useful for actually getting records, nor does it need to be because other methods are handling that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed. Thanks for your review.

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

i think this change makes sense, lgtm 👍

@zhangyue19921010
Copy link
Contributor Author

Hi @clintropolis Thanks for your review and approval! Job 75 (Compile=openjdk8, Run=openjdk11) leadership and high availability integration tests is failed. But I think job 75 is not this pr related. Retry may be passed :)

@suneet-s
Copy link
Contributor

@zhangyue19921010 have you been able to run the kinesis integration tests locally to see if those tests continue to pass after this change? There are some instructions on how to run them here - https://github.com/apache/druid/blob/master/integration-tests/README.md#running-a-test-that-uses-cloud

@zhangyue19921010
Copy link
Contributor Author

Hi @suneet-s Thanks for reminding. I just run the kinesis integration tests locally and succeeded.

屏幕快照 2021-03-18 下午3 02 46

@suneet-s
Copy link
Contributor

Thanks for the contribution @zhangyue19921010 !

@suneet-s suneet-s merged commit 8b4f966 into apache:master Mar 19, 2021
@zhangyue19921010 zhangyue19921010 deleted the kinesis-lag-metrics-bug-fix branch March 20, 2021 02:23
@zhangyue19921010
Copy link
Contributor Author

Thanks a lot :) @suneet-s and @clintropolis

@clintropolis clintropolis added this to the 0.22.0 milestone Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lag keep increasing when there is no more new data for kinesis.

3 participants