You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/03/25 22:41:49 UTC
[GitHub] [cassandra] rustyrazorblade opened a new pull request #488:
CASSANDRA-15661
rustyrazorblade opened a new pull request #488: CASSANDRA-15661
URL: https://github.com/apache/cassandra/pull/488
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r400332537
##########
File path: src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
##########
@@ -530,8 +529,7 @@ public static SSTableReader open(Descriptor descriptor,
}
long fileLength = new File(descriptor.filenameFor(Component.DATA)).length();
- if (logger.isDebugEnabled())
- logger.debug("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
+ logger.info("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
Review comment:
I can't think of a time where this would've been useful to diagnose an issue, which doesn't mean such a case doesn't exist.
My 2 cents on this is that so many lines will bother me as startup logging is already a lot, but I know what to search if I want to move quickly to a specific line, so I can deal with it.
If you have cases in mind where this would be helpful, fair enough.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399195853
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java
##########
@@ -253,7 +253,7 @@ public void readCommitLogSegment(CommitLogReadHandler handler,
throw (IOException) re.getCause();
throw re;
}
- logger.debug("Finished reading {}", file);
+ logger.info("Finished reading {}", file);
Review comment:
For which particular case was the logging for a finished commit log read bumped to INFO?
I can't think of cases when this would have helped me troubleshoot a cluster.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399194261
##########
File path: src/java/org/apache/cassandra/transport/ConnectionLimitHandler.java
##########
@@ -61,7 +61,7 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception
if (count > limit)
{
// The decrement will be done in channelClosed(...)
- noSpamLogger.warn("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
+ noSpamLogger.error("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
Review comment:
I'm not sure how to use the noSpamLogger globally, because it looks like loggings that have been confusing folks a lot. You see error messages there that you can ignore (like the one with allocation for the chunk cache being impossible, which is not something that's really actionable).
What's the intent behind the noSpamLogger and what are operators supposed to do about the messages you're raising from warn to error here?
Is that ConnectionLimitHandler used in incremental repair?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399447394
##########
File path: src/java/org/apache/cassandra/transport/ConnectionLimitHandler.java
##########
@@ -61,7 +61,7 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception
if (count > limit)
{
// The decrement will be done in channelClosed(...)
- noSpamLogger.warn("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
+ noSpamLogger.error("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
Review comment:
The nospam logger has some logic to prevent flooding logs with too much information. The comment on the class says this:
```
Logging that limits each log statement to firing based on time since the statement last fired.
```
You'd see this error if you had misconfigured your client to make too many connections to the DB. For example if you did `.setConnectionsPerHost(1000)`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399471424
##########
File path: src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
##########
@@ -530,8 +529,7 @@ public static SSTableReader open(Descriptor descriptor,
}
long fileLength = new File(descriptor.filenameFor(Component.DATA)).length();
- if (logger.isDebugEnabled())
- logger.debug("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
+ logger.info("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
Review comment:
I ran a test where I ran tlp-stress against my node locally with compaction disabled, and created 3K sstables, then started things back up.
Here's a search of the system log for the opening lines:
```
rg Opening system.log | wc -l
3030
```
I saved the log messages themselves to a separate file:
```
rg Opening system.log > opening.log
```
The total size of the file is 615KB. Given the importance of being able to diagnose data loss problems after the fact, I'd say this is a pretty small price to pay for the ability to go back and figure out if the server started up correctly. What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399194945
##########
File path: src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
##########
@@ -530,8 +529,7 @@ public static SSTableReader open(Descriptor descriptor,
}
long fileLength = new File(descriptor.filenameFor(Component.DATA)).length();
- if (logger.isDebugEnabled())
- logger.debug("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
+ logger.info("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
Review comment:
The startup log could get clogged really quickly if there are tables with a lot of files (like those using LCS, which can end up with thousands of sstables), making it hard to read.
What would you need that information for at INFO level?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399194643
##########
File path: src/java/org/apache/cassandra/repair/consistent/LocalSessions.java
##########
@@ -277,7 +277,7 @@ else if (shouldDelete(session, now))
{
if (!sessionHasData(session))
{
- logger.debug("Auto deleting repair session {}", session);
+ logger.info("Auto deleting repair session {}", session);
Review comment:
The loggings in this class should probably remain at debug level IMO.
These messages are really useful to debug things that could go wrong in how incremental repair is implemented.
While I support getting back memtable flushes and compaction loggings in system.log, these lines will hardly be useful to most operators (but I understand they are to Cassandra devs).
wdyt?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r400332707
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java
##########
@@ -253,7 +253,7 @@ public void readCommitLogSegment(CommitLogReadHandler handler,
throw (IOException) re.getCause();
throw re;
}
- logger.debug("Finished reading {}", file);
+ logger.info("Finished reading {}", file);
Review comment:
kk
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r400335467
##########
File path: src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
##########
@@ -530,8 +529,7 @@ public static SSTableReader open(Descriptor descriptor,
}
long fileLength = new File(descriptor.filenameFor(Component.DATA)).length();
- if (logger.isDebugEnabled())
- logger.debug("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
+ logger.info("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
Review comment:
I ran into a few cases at TLP were folks were concerned data was missing. My diagnosis after the fact was difficult because I didn't know what SSTables had been opened.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399467498
##########
File path: src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
##########
@@ -530,8 +529,7 @@ public static SSTableReader open(Descriptor descriptor,
}
long fileLength = new File(descriptor.filenameFor(Component.DATA)).length();
- if (logger.isDebugEnabled())
- logger.debug("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
+ logger.info("Opening {} ({})", descriptor, FBUtilities.prettyPrintMemory(fileLength));
Review comment:
Yes, you could have a few thousand lines of logs, but in the grand scheme of things that's pretty minor compared to the benefit of knowing if the DB started up correctly. After startup you wouldn't see this very often. This goes back to the idea of putting INFO logging in to be able to understand correctness after the fact. If the DB starts up and you don't have this information readily available it's hard to answer questions like "did Cassandra see SSTable XYZ?" I can think of a few cases in the past when helping teams with questions of data loss that this has come up.
I've run a server with this logging enabled, and after startup it's rarely seen. I think it's an important part of the startup checks. I'd definitely want it on any servers that I would later try to diagnose with a potential data loss issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399450292
##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogReader.java
##########
@@ -253,7 +253,7 @@ public void readCommitLogSegment(CommitLogReadHandler handler,
throw (IOException) re.getCause();
throw re;
}
- logger.debug("Finished reading {}", file);
+ logger.info("Finished reading {}", file);
Review comment:
After a non-graceful restart. There are cases where the segment can get skipped and data missed, this can help identify data loss.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399448422
##########
File path: src/java/org/apache/cassandra/repair/consistent/LocalSessions.java
##########
@@ -277,7 +277,7 @@ else if (shouldDelete(session, now))
{
if (!sessionHasData(session))
{
- logger.debug("Auto deleting repair session {}", session);
+ logger.info("Auto deleting repair session {}", session);
Review comment:
If we were seeing this a lot, I'd agree with you, but the amount of logging that will be added to INFO is relatively small, compared with the benefit of understanding what's happening if something goes wrong. The case here is to understand the problem after the fact. Since incremental repair is essentially a full rewrite in 4.0, I'm leaning on the side of "a few extra log messages can prove to be very helpful" rather than "maybe people won't need it.".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r401278328
##########
File path: src/java/org/apache/cassandra/transport/ConnectionLimitHandler.java
##########
@@ -61,7 +61,7 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception
if (count > limit)
{
// The decrement will be done in channelClosed(...)
- noSpamLogger.warn("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
+ noSpamLogger.error("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
Review comment:
OK. I've added some extra information to the logs which should make the origin a little more clear.
One thing to note is that this message can only occur if someone has explicitly set native_transport_max_concurrent_connections, or native_transport_max_concurrent_connections_per_ip.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r400324602
##########
File path: src/java/org/apache/cassandra/repair/consistent/LocalSessions.java
##########
@@ -277,7 +277,7 @@ else if (shouldDelete(session, now))
{
if (!sessionHasData(session))
{
- logger.debug("Auto deleting repair session {}", session);
+ logger.info("Auto deleting repair session {}", session);
Review comment:
ok, works for me then.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adejanovski commented on a change in pull request #488:
CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
adejanovski commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r400324351
##########
File path: src/java/org/apache/cassandra/transport/ConnectionLimitHandler.java
##########
@@ -61,7 +61,7 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception
if (count > limit)
{
// The decrement will be done in channelClosed(...)
- noSpamLogger.warn("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
+ noSpamLogger.error("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
Review comment:
ok gotcha.
But if I run into such a log message, what should I do as an operator?
Maybe we could give some pointers on how to remediate the situation? Do you have some safe recommendations to make here (like the driver config you mentioned).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rustyrazorblade commented on a change in pull request
#488: CASSANDRA-15661 improve logging
Posted by GitBox <gi...@apache.org>.
rustyrazorblade commented on a change in pull request #488: CASSANDRA-15661 improve logging
URL: https://github.com/apache/cassandra/pull/488#discussion_r399447394
##########
File path: src/java/org/apache/cassandra/transport/ConnectionLimitHandler.java
##########
@@ -61,7 +61,7 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception
if (count > limit)
{
// The decrement will be done in channelClosed(...)
- noSpamLogger.warn("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
+ noSpamLogger.error("Exceeded maximum native connection limit of {} by using {} connections", limit, count);
Review comment:
The nospam logger has some logic to prevent flooding logs with too much information. The comment on the class says this:
```
Logging that limits each log statement to firing based on time since the statement last fired.
```
You'd see this if you had misconfigured your client to make too many connections to the DB. For example if you did `.setConnectionsPerHost(1000)`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org