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

move from error to warn #142

Merged
merged 5 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions helm/templates/broker/log-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,24 @@ metadata:
heritage: {{ .Release.Service }}
data:
pinot-broker-log4j2.xml: |-
<Configuration>
<Configuration monitorInterval="30">
<Appenders>
<Console name="console" target="SYSTEM_OUT">
<PatternLayout>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1}] [%t] %m%n</pattern>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1.}] [%t] %m%n</pattern>
</PatternLayout>
</Console>
</Appenders>
<Loggers>
<Root level={{ .Values.broker.logging.level | quote }} additivity="false">
<!-- Display most logs on the console -->
<AppenderRef ref="console"/>
</Root>
<AsyncLogger name="org.reflections" level="error" additivity="false"/>
<AsyncLogger name="org.apache.pinot.broker.querylog.QueryLogger" level="info"/>
<AsyncLogger name="org.reflections" level="error" additivity="false">
<AppenderRef ref="console"/>
</AsyncLogger>
<Logger name="org.apache.pinot.broker.querylog.QueryLogger" level="info" additivity="false">
<AppenderRef ref="console"/>
</Logger>
</Loggers>
</Configuration>
{{- end }}
12 changes: 8 additions & 4 deletions helm/templates/controller/log-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,25 @@ metadata:
heritage: {{ .Release.Service }}
data:
pinot-controller-log4j2.xml: |-
<Configuration>
<Configuration monitorInterval="30">
Copy link
Contributor Author

@kishansairam9 kishansairam9 Feb 8, 2024

Choose a reason for hiding this comment

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

same with this, we will be modifying in helm-values if overriden

<Appenders>
<Console name="console" target="SYSTEM_OUT">
<PatternLayout>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1}] [%t] %m%n</pattern>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1.}] [%t] %m%n</pattern>
</PatternLayout>
</Console>

</Appenders>
<Loggers>
<Root level={{ .Values.controller.logging.level | quote }} additivity="false">
<!-- Display most logs on the console -->
<AppenderRef ref="console"/>
</Root>
<AsyncLogger name="org.reflections" level="error" additivity="false"/>
<AsyncLogger name="org.reflections" level="error" additivity="false">
<AppenderRef ref="console"/>
</AsyncLogger>
<Logger name="org.apache.pinot.shaded.org.apache.kakfa" level="error" additivity="false">
<AppenderRef ref="console"/>
</Logger>
</Loggers>
</Configuration>
{{- end }}
9 changes: 5 additions & 4 deletions helm/templates/minion/log-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,21 @@ metadata:
heritage: {{ .Release.Service }}
data:
pinot-minion-log4j2.xml: |-
<Configuration>
<Configuration monitorInterval="30">
<Appenders>
<Console name="console" target="SYSTEM_OUT">
<PatternLayout>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1}] [%t] %m%n</pattern>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1.}] [%t] %m%n</pattern>
</PatternLayout>
</Console>
</Appenders>
<Loggers>
<Root level={{ .Values.minion.logging.level | quote }} additivity="false">
<!-- Display most logs on the console -->
<AppenderRef ref="console"/>
</Root>
<AsyncLogger name="org.reflections" level="error" additivity="false"/>
<AsyncLogger name="org.reflections" level="error" additivity="false">
<AppenderRef ref="console"/>
</AsyncLogger>
</Loggers>
</Configuration>
{{- end }}
13 changes: 7 additions & 6 deletions helm/templates/server/log-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,23 @@ metadata:
heritage: {{ .Release.Service }}
data:
pinot-server-log4j2.xml: |-
<Configuration>
<Configuration monitorInterval="30">
<Appenders>
<Console name="console" target="SYSTEM_OUT">
<PatternLayout>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1}] [%t] %m%n</pattern>
<pattern>%d{yyyy/MM/dd HH:mm:ss.SSS} %p [%c{1.}] [%t] %m%n</pattern>
</PatternLayout>
</Console>
</Appenders>
<Loggers>
<Root level={{ .Values.server.logging.level | quote }} additivity="false">
<!-- Display most logs on the console -->
<AppenderRef ref="console"/>
</Root>
<AsyncLogger name="org.reflections" level="error" additivity="false"/>
<Logger name="org.apache.kafka">
<level value="error" />
<AsyncLogger name="org.reflections" level="error" additivity="false">
<AppenderRef ref="console"/>
</AsyncLogger>
<Logger name="org.apache.pinot.shaded.org.apache.kakfa" level="error" additivity="false">
<AppenderRef ref="console"/>
</Logger>
</Loggers>
</Configuration>
Expand Down
8 changes: 4 additions & 4 deletions helm/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ controller:
namespace: monitoring

logging:
level: error
level: warn

broker:
name: broker
Expand Down Expand Up @@ -278,7 +278,7 @@ broker:
memory: "256Mi"

logging:
level: error
level: warn

minion:
name: minion
Expand Down Expand Up @@ -386,7 +386,7 @@ minion:
memory: "256Mi"

logging:
level: error
level: warn

server:
name: server
Expand Down Expand Up @@ -508,7 +508,7 @@ server:
type: pd-standard

logging:
level: error
level: warn
Copy link
Contributor

Choose a reason for hiding this comment

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

it already is set to error in helm-values repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will adjust in helm-values repo as needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

what I meant was that we already have set the logging level to error. are there any specific warn logs we are interested in? there were too much logging by pinot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Laxman and mihir were of opinion we should have warn logs and fix any underlying issues

Copy link
Contributor

Choose a reason for hiding this comment

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

I only suggested better to keep warn as the default.
If they are too noisy, we may need to investigate and fix or turn them off at package level in worst case.


tiers:
default: {}
Expand Down
Loading