You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "summerhasama-stripe (via GitHub)" <gi...@apache.org> on 2023/06/27 20:06:51 UTC
[GitHub] [pinot] summerhasama-stripe opened a new pull request, #10983: Message batch ingestion lag fix
summerhasama-stripe opened a new pull request, #10983:
URL: https://github.com/apache/pinot/pull/10983
We saw a lot of logs for the FreshnessBasedConsumptionStatusChecker that show the servers are not aware of the freshness of incoming events
Through some debugging, we determined the issue was the stream ingestion interface changed a few months ago, and the backwards compatible code actually has a bug.
- "getStreamMessage" default implementation fails to provide the metadata for the message
- The new stream message metadata interface doesn’t have a 1 arg constructor anymore
Tested: We deployed this change and we see the consumption checker now has freshness:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #10983: Message batch ingestion lag fix
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10983:
URL: https://github.com/apache/pinot/pull/10983#discussion_r1244326440
##########
pinot-spi/src/test/java/org/apache/pinot/spi/stream/StreamDataDecoderImplTest.java:
##########
@@ -41,7 +41,7 @@ public void testDecodeValueOnly()
messageDecoder.init(Collections.emptyMap(), ImmutableSet.of(NAME_FIELD), "");
String value = "Alice";
StreamMessage<byte[]> message = new StreamMessage(value.getBytes(StandardCharsets.UTF_8),
- value.getBytes(StandardCharsets.UTF_8).length);
+ value.getBytes(StandardCharsets.UTF_8).length, null);
Review Comment:
can we add a test with non-null metadata?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamMessage.java:
##########
@@ -51,8 +51,8 @@ public StreamMessage(@Nullable byte[] key, T value, @Nullable StreamMessageMetad
_length = length;
}
- public StreamMessage(T value, int length) {
- this(null, value, null, length);
+ public StreamMessage(T value, int length, @Nullable StreamMessageMetadata metadata) {
+ this(null, value, metadata, length);
Review Comment:
let's keep original and add a nullable constructor?
```suggestion
public StreamMessage(T value, int length) {
this(value, length, null);
}
public StreamMessage(T value, int length, @Nullable StreamMessageMetadata metadata) {
this(null, value, metadata, length);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] summerhasama-stripe commented on pull request #10983: Message batch ingestion lag fix
Posted by "summerhasama-stripe (via GitHub)" <gi...@apache.org>.
summerhasama-stripe commented on PR #10983:
URL: https://github.com/apache/pinot/pull/10983#issuecomment-1613211565
@walterddr Can you rerun the failed checks, they seem like random errors?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] summerhasama-stripe commented on a diff in pull request #10983: Message batch ingestion lag fix
Posted by "summerhasama-stripe (via GitHub)" <gi...@apache.org>.
summerhasama-stripe commented on code in PR #10983:
URL: https://github.com/apache/pinot/pull/10983#discussion_r1245377750
##########
pinot-spi/src/test/java/org/apache/pinot/spi/stream/StreamDataDecoderImplTest.java:
##########
@@ -41,7 +41,7 @@ public void testDecodeValueOnly()
messageDecoder.init(Collections.emptyMap(), ImmutableSet.of(NAME_FIELD), "");
String value = "Alice";
StreamMessage<byte[]> message = new StreamMessage(value.getBytes(StandardCharsets.UTF_8),
- value.getBytes(StandardCharsets.UTF_8).length);
+ value.getBytes(StandardCharsets.UTF_8).length, null);
Review Comment:
Yes, this test seems to be focused on the StreamDataDecoder so I added to a different unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10983: Message batch ingestion lag fix
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10983:
URL: https://github.com/apache/pinot/pull/10983#issuecomment-1610245244
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10983?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10983](https://app.codecov.io/gh/apache/pinot/pull/10983?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ef346b5) into [master](https://app.codecov.io/gh/apache/pinot/commit/2d30e28bedea29d264b0d7946ebb3f0e2cf1b5cc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (2d30e28) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10983 +/- ##
==========================================
- Coverage 0.11% 0.11% -0.01%
==========================================
Files 2190 2190
Lines 118056 118058 +2
Branches 17873 17873
==========================================
Hits 137 137
- Misses 117899 117901 +2
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `0.00% <0.00%> (ø)` | |
| integration1temurin20 | `0.00% <0.00%> (ø)` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin11 | `0.00% <0.00%> (ø)` | |
| unittests1temurin17 | `0.00% <0.00%> (ø)` | |
| unittests1temurin20 | `0.00% <0.00%> (ø)` | |
| unittests2temurin11 | `0.11% <0.00%> (ø)` | |
| unittests2temurin17 | `0.11% <0.00%> (ø)` | |
| unittests2temurin20 | `0.11% <0.00%> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10983?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...java/org/apache/pinot/spi/stream/MessageBatch.java](https://app.codecov.io/gh/apache/pinot/pull/10983?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL01lc3NhZ2VCYXRjaC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ava/org/apache/pinot/spi/stream/StreamMessage.java](https://app.codecov.io/gh/apache/pinot/pull/10983?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1N0cmVhbU1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...apache/pinot/spi/stream/StreamMessageMetadata.java](https://app.codecov.io/gh/apache/pinot/pull/10983?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1N0cmVhbU1lc3NhZ2VNZXRhZGF0YS5qYXZh) | `0.00% <0.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr merged pull request #10983: Message batch ingestion lag fix
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #10983:
URL: https://github.com/apache/pinot/pull/10983
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org