[GitHub] [pinot] navina commented on pull request #10136: Decouple dependency on using peer segment download scheme for doing metadata pushes to controller

navina commented on PR #10136: URL: https://github.com/apache/pinot/pull/10136#issuecomment-1398019400 > Please check the test failure. I think it is related to the current change Yes will fix it. Ty -- This is an automated message from the Apache Git Service. To respond to the mess

[GitHub] [pinot] codecov-commenter commented on pull request #10155: Make a system property to call system.exit() for LaunchDataIngestionJobCommand

codecov-commenter commented on PR #10155: URL: https://github.com/apache/pinot/pull/10155#issuecomment-1398016275 # [Codecov](https://codecov.io/gh/apache/pinot/pull/10155?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Soft

[GitHub] [pinot] xiangfu0 opened a new pull request, #10155: Make a system property to call system.exit() for LaunchDataIngestionJobCommand

xiangfu0 opened a new pull request, #10155: URL: https://github.com/apache/pinot/pull/10155 Use a system property to call system.exit() for LaunchDataIngestionJobCommand. The reason is SparkJob may fail due to this system.exit() call. -- This is an automated message from the Ap

[GitHub] [pinot] codecov-commenter commented on pull request #10154: [multistage] make mailboxID JSON serialized

codecov-commenter commented on PR #10154: URL: https://github.com/apache/pinot/pull/10154#issuecomment-1397915490 # [Codecov](https://codecov.io/gh/apache/pinot/pull/10154?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Soft

[GitHub] [pinot] agavra opened a new pull request, #10154: [multistage] make mailboxID JSON serialized

agavra opened a new pull request, #10154: URL: https://github.com/apache/pinot/pull/10154 Minor PR to change the serialization of mailbox identifier to a JSON format. For example: ``` {"jobId":"-7324288832996730535_2","from":"localhost:52683","to":"localhost:52683"} ``` This

[GitHub] [pinot] codecov-commenter commented on pull request #10153: allow editing pools in instance details ui

codecov-commenter commented on PR #10153: URL: https://github.com/apache/pinot/pull/10153#issuecomment-139776 # [Codecov](https://codecov.io/gh/apache/pinot/pull/10153?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Soft

[GitHub] [pinot] jadami10 opened a new pull request, #10153: allow editing pools in instance details ui

jadami10 opened a new pull request, #10153: URL: https://github.com/apache/pinot/pull/10153 This is a simple ui feature + bugfix. Today there's two issues: 1) there's no way from the UI to easily create or update pools from the instance details config page 2) if you are using pools, an

[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10151: Simplify UTF-8 comparison on null character

Jackie-Jiang commented on code in PR #10151: URL: https://github.com/apache/pinot/pull/10151#discussion_r1082047825 ## pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readerwriter/ValueReaderComparisonTest.java: ## @@ -156,22 +156,27 @@ void testSu

[GitHub] [pinot] jadami10 commented on pull request #9994: [feature] Add a Tracker class to support aggregate-worst case consumption delay…

jadami10 commented on PR #9994: URL: https://github.com/apache/pinot/pull/9994#issuecomment-1397833536 I tried deploying this change to one of our clusters with #10101 or #10121. We currently track ingestion lag via our custom stream ingestion plugin. For the most part these match up. But I

[GitHub] [pinot] Jackie-Jiang commented on pull request #10151: Simplify UTF-8 comparison on null character

Jackie-Jiang commented on PR #10151: URL: https://github.com/apache/pinot/pull/10151#issuecomment-139782 With the current code, `length == bytes.length && length != 0 && bytes[length - 1] == 0` case is not handled (the new added `ValueReaderComparisonTest` is flaky right now). To keep t

[GitHub] [pinot] sajjad-moradi commented on a diff in pull request #10121: Add a tracker for end-to-end consumption delay of events.

sajjad-moradi commented on code in PR #10121: URL: https://github.com/apache/pinot/pull/10121#discussion_r1082035842 ## pinot-spi/src/main/java/org/apache/pinot/spi/stream/RowMetadata.java: ## @@ -49,6 +49,17 @@ public interface RowMetadata { */ long getRecordIngestionTim

[GitHub] [pinot] sajjad-moradi commented on a diff in pull request #10121: Add a tracker for end-to-end consumption delay of events.

sajjad-moradi commented on code in PR #10121: URL: https://github.com/apache/pinot/pull/10121#discussion_r1082029067 ## pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java: ## @@ -76,6 +76,15 @@ public class IngestionDelayTracker {

[GitHub] [pinot] Jackie-Jiang closed pull request #10144: Adding duration unit in trivy timeout

Jackie-Jiang closed pull request #10144: Adding duration unit in trivy timeout URL: https://github.com/apache/pinot/pull/10144 -- 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. T

[GitHub] [pinot] Jackie-Jiang commented on pull request #10144: Adding duration unit in trivy timeout

Jackie-Jiang commented on PR #10144: URL: https://github.com/apache/pinot/pull/10144#issuecomment-1397822649 This change is included in #10150 which also fixes the trivy failure -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

[pinot] branch master updated: Fix json-smart version (#10150)

This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git The following commit(s) were added to refs/heads/master by this push: new 4d7da0abab Fix json-smart version (#10150) 4d7da0

[GitHub] [pinot] Jackie-Jiang merged pull request #10150: Fix json-smart version

Jackie-Jiang merged PR #10150: URL: https://github.com/apache/pinot/pull/10150 -- 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-unsubscr...@pinot

[GitHub] [pinot] mcvsubbu commented on issue #10147: Pauseless Consumption

mcvsubbu commented on issue #10147: URL: https://github.com/apache/pinot/issues/10147#issuecomment-1397819480 How many rows would the new consuming segment be setup to consume? (Currently that is decided when the prev segment is committed). -- This is an automated message from the Apache

[GitHub] [pinot] Jackie-Jiang commented on pull request #10139: JMX to Prom Exporter - More Inclusive Table Regex

Jackie-Jiang commented on PR #10139: URL: https://github.com/apache/pinot/pull/10139#issuecomment-1397807868 Whitespaces in table name is definitely not recommended, but I don't think we reject it right now. Actually it is also not recommended to have '-' in table name or column name becaus

[GitHub] [pinot] Jackie-Jiang commented on issue #10147: Pauseless Consumption

Jackie-Jiang commented on issue #10147: URL: https://github.com/apache/pinot/issues/10147#issuecomment-1397794076 Have you considered using the regular state transition to achieve this? Essentially we can create the new consuming segment in IdealState when the commit starts. Nothing needs t

[GitHub] [pinot] Jackie-Jiang commented on pull request #10151: Simplify UTF-8 comparison on null character

Jackie-Jiang commented on PR #10151: URL: https://github.com/apache/pinot/pull/10151#issuecomment-1397787608 > Does `LoaderTest.testPadding` pass with this change? I'm all for simplification (assuming whatever the motivation for adding `LoaderTest.testPadding` was no longer matters) but thi

[GitHub] [pinot] navina opened a new issue, #10152: Support consumer de-aggregation in Kinesis

navina opened a new issue, #10152: URL: https://github.com/apache/pinot/issues/10152 Kinesis supports producing aggregated (aka batched) record. Thus, kinesis consumer also has support for de-aggregating the records. Refer - https://docs.aws.amazon.com/streams/latest/dev/kinesis-kpl-consume

[GitHub] [pinot] richardstartin commented on pull request #10151: Simplify UTF-8 comparison on null character

richardstartin commented on PR #10151: URL: https://github.com/apache/pinot/pull/10151#issuecomment-1397748773 Does `LoaderTest.testPadding` pass with this change? I'm all for simplification (assuming the whatever the motivation for adding `LoaderTest.testPadding` was no longer matters) but

[GitHub] [pinot] walterddr commented on a diff in pull request #10151: Simplify UTF-8 comparison on null character

walterddr commented on code in PR #10151: URL: https://github.com/apache/pinot/pull/10151#discussion_r1081975613 ## pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readerwriter/ValueReaderComparisonTest.java: ## @@ -156,22 +156,27 @@ void testSuper

[GitHub] [pinot] navina commented on a diff in pull request #10136: Decouple dependency on using peer segment download scheme for doing metadata pushes to controller

navina commented on code in PR #10136: URL: https://github.com/apache/pinot/pull/10136#discussion_r1081975469 ## pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java: ## @@ -49,9 +49,13 @@ public class SegmentsValidationAndRetention

[GitHub] [pinot] codecov-commenter commented on pull request #10151: Simplify UTF-8 comparison on null character

codecov-commenter commented on PR #10151: URL: https://github.com/apache/pinot/pull/10151#issuecomment-1397743731 # [Codecov](https://codecov.io/gh/apache/pinot/pull/10151?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Soft

[pinot] branch master updated: [multistage] [feature] Add a query option to pass some v1 limit (#9957)

This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git The following commit(s) were added to refs/heads/master by this push: new fbd673482b [multistage] [feature] Add a query opt

[GitHub] [pinot] Jackie-Jiang merged pull request #9957: [multistage] [feature] Add a query option to pass some v1 limit

Jackie-Jiang merged PR #9957: URL: https://github.com/apache/pinot/pull/9957 -- 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-unsubscr...@pinot.a

[GitHub] [pinot] Jackie-Jiang opened a new pull request, #10151: Simplify UTF-8 comparison on null character

Jackie-Jiang opened a new pull request, #10151: URL: https://github.com/apache/pinot/pull/10151 Null character (byte zero) is not allowed in string values ingested into Pinot (all the string values ingested will be sanitized and all the characters after the first null character will be trun

[GitHub] [pinot] agavra commented on a diff in pull request #10120: [multistage] support aggregations that require intermediate representations

agavra commented on code in PR #10120: URL: https://github.com/apache/pinot/pull/10120#discussion_r1081932230 ## pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotReduceAggregateFunctionsRule.java: ## @@ -0,0 +1,201 @@ +/** + * Licensed to the Apache Software F

[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10136: Decouple dependency on using peer segment download scheme for doing metadata pushes to controller

Jackie-Jiang commented on code in PR #10136: URL: https://github.com/apache/pinot/pull/10136#discussion_r1081822321 ## pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java: ## @@ -49,9 +49,13 @@ public class SegmentsValidationAndRet

[GitHub] [pinot] agavra commented on a diff in pull request #10120: [multistage] support aggregations that require intermediate representations

agavra commented on code in PR #10120: URL: https://github.com/apache/pinot/pull/10120#discussion_r1081825016 ## pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java: ## @@ -65,6 +65,7 @@ public enum AggregationFunctionType { STDDEVSAMP("s

[GitHub] [pinot] navina commented on pull request #10144: Adding duration unit in trivy timeout

navina commented on PR #10144: URL: https://github.com/apache/pinot/pull/10144#issuecomment-1397475658 > I understand that the initial idea was to increase the timeout but we are using this PR to change the way trivy is triggered. Therefore I would suggest to change the title of the PR

[GitHub] [pinot] saurabhd336 commented on a diff in pull request #10089: Reload conditional skip

saurabhd336 commented on code in PR #10089: URL: https://github.com/apache/pinot/pull/10089#discussion_r1081698660 ## pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java: ## @@ -403,6 +417,16 @@ public void reloadSegment(String segmentName, Ind

[GitHub] [pinot] saurabhd336 commented on a diff in pull request #10089: Reload conditional skip

saurabhd336 commented on code in PR #10089: URL: https://github.com/apache/pinot/pull/10089#discussion_r1081688733 ## pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerTest.java: ## @@ -191,7 +191,6 @@ public void testReloadSegmentUseLocalCopy()

[GitHub] [pinot] walterddr commented on issue #10129: [multistage] Projection Not Pushed Down for Agg Queries with No Grouping Sets

walterddr commented on issue #10129: URL: https://github.com/apache/pinot/issues/10129#issuecomment-1397438501 did a bit of research on this topic: the problem only occurs on `COUNT(*)` or `COUNT(1)`, `COUNT(col)` this is b/c calcite will optimize out the argument since all columns in

[GitHub] [pinot] 61yao commented on a diff in pull request #9957: [multistage] [feature] Add a query option to pass some v1 limit

61yao commented on code in PR #9957: URL: https://github.com/apache/pinot/pull/9957#discussion_r1081642038 ## pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java: ## @@ -99,6 +99,14 @@ public void testGeneratedQueries()

[GitHub] [pinot] snleee merged pull request #10106: clean up the logic of reading guage metrics

snleee merged PR #10106: URL: https://github.com/apache/pinot/pull/10106 -- 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-unsubscr...@pinot.apach

[pinot] branch master updated: clean up the logic of reading guage metrics (#10106)

This is an automated email from the ASF dual-hosted git repository. snlee pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git The following commit(s) were added to refs/heads/master by this push: new d4356b47ed clean up the logic of reading guage met

[GitHub] [pinot] npawar commented on pull request #10144: Adding duration unit in trivy timeout

npawar commented on PR #10144: URL: https://github.com/apache/pinot/pull/10144#issuecomment-1397373177 Not sure if periodic task is the best way to go forward here. I'm okay if we merge the flagged PR, and then separately fix the vulnerability detected (either the same PR author does it, or

[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics

zhtaoxiang commented on code in PR #10106: URL: https://github.com/apache/pinot/pull/10106#discussion_r1081609829 ## pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java: ## @@ -481,20 +461,6 @@ public long getValueOfTableGauge(final String tableName,

[GitHub] [pinot] zhtaoxiang commented on a diff in pull request #10106: clean up the logic of reading guage metrics

zhtaoxiang commented on code in PR #10106: URL: https://github.com/apache/pinot/pull/10106#discussion_r1081607958 ## pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java: ## @@ -0,0 +1,128 @@ +/** + * Licensed to the Apache Software Foundation (ASF) u

[GitHub] [pinot] klsince commented on a diff in pull request #10089: Reload conditional skip

klsince commented on code in PR #10089: URL: https://github.com/apache/pinot/pull/10089#discussion_r1081607056 ## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java: ## @@ -207,11 +207,7 @@ private boolean needProcessSt

[GitHub] [pinot] walterddr commented on pull request #10148: [multistage] flag all columns as nullable

walterddr commented on PR #10148: URL: https://github.com/apache/pinot/pull/10148#issuecomment-1397353721 this have introduce new set of issues. so we really need a proper null detection on columns. will shelf this PR for now -- This is an automated message from the Apache Git Service. To

[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics

jugomezv commented on code in PR #10106: URL: https://github.com/apache/pinot/pull/10106#discussion_r1081597788 ## pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java: ## @@ -0,0 +1,128 @@ +/** + * Licensed to the Apache Software Foundation (ASF) und

[GitHub] [pinot] jugomezv commented on a diff in pull request #10106: clean up the logic of reading guage metrics

jugomezv commented on code in PR #10106: URL: https://github.com/apache/pinot/pull/10106#discussion_r1081595797 ## pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java: ## @@ -481,20 +461,6 @@ public long getValueOfTableGauge(final String tableName, f

[pinot] branch master updated: [multistage] [debuggability] OpChain and operator stats (#10094)

This is an automated email from the ASF dual-hosted git repository. rongr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git The following commit(s) were added to refs/heads/master by this push: new 5416652ff5 [multistage] [debuggability] OpChain an

[GitHub] [pinot] walterddr merged pull request #10094: [multistage] [debuggability] OpChain and operator stats

walterddr merged PR #10094: URL: https://github.com/apache/pinot/pull/10094 -- 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-unsubscr...@pinot.ap

[GitHub] [pinot] walterddr commented on a diff in pull request #10144: Adding duration unit in trivy timeout

walterddr commented on code in PR #10144: URL: https://github.com/apache/pinot/pull/10144#discussion_r1081538383 ## .github/workflows/pinot_vuln_check.yml: ## @@ -19,18 +19,8 @@ name: Pinot Dependencies on: - push: -branches: - - master - pull_request: -branch

[GitHub] [pinot] ankitsultana commented on a diff in pull request #10148: [multistage] flag all columns as nullable

ankitsultana commented on code in PR #10148: URL: https://github.com/apache/pinot/pull/10148#discussion_r1081312765 ## pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java: ## @@ -54,23 +54,23 @@ public RelDataType createRelDataTypeFromSchema(Schema sc

[GitHub] [pinot] ankitsultana commented on issue #10140: [multistage] Expensive RoundRobinScheduler::hasNext Calls

ankitsultana commented on issue #10140: URL: https://github.com/apache/pinot/issues/10140#issuecomment-1396973491 Side-note: I think entries in seenMail may get leaked if `_releaseTs` is set to a finite value. The reason is that the OpChain may hang up and the sender may send the data after

[GitHub] [pinot] Ferix9288 commented on pull request #10139: JMX to Prom Exporter - More Inclusive Table Regex

Ferix9288 commented on PR #10139: URL: https://github.com/apache/pinot/pull/10139#issuecomment-1396918251 Thanks @richardstartin for the input! @Jackie-Jiang I added `\\S+` regex to all other instances in which there was a `table` label; good catch on that. Also are we sure we should

[pinot] branch master updated: Fixing the metadata tar gz file path (#10149)

This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git The following commit(s) were added to refs/heads/master by this push: new 112893bd6e Fixing the metadata tar gz file path

[GitHub] [pinot] xiangfu0 merged pull request #10149: Fixing the metadata tar gz file path

xiangfu0 merged PR #10149: URL: https://github.com/apache/pinot/pull/10149 -- 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-unsubscr...@pinot.apa

[GitHub] [pinot] richardstartin commented on pull request #10139: JMX to Prom Exporter - More Inclusive Table Regex

richardstartin commented on PR #10139: URL: https://github.com/apache/pinot/pull/10139#issuecomment-1396760434 I don't have much of an opinion on what these regular expressions should be, I just took what was there before (which was working for us) and split it into different files so we do

[GitHub] [pinot] Ferix9288 commented on pull request #10139: JMX to Prom Exporter - More Inclusive Table Regex

Ferix9288 commented on PR #10139: URL: https://github.com/apache/pinot/pull/10139#issuecomment-1396699401 Hey @Jackie-Jiang , much appreciated for the prompt review! Ah very nice. So possibly a non-greedy search like (.*?)_(\w+)? And np, I will fix the other regexes that also contain