yugabyte / yugabyte-db

YugabyteDB - the cloud native distributed SQL database for mission-critical applications.
https://www.yugabyte.com
Other
8.96k stars 1.07k forks source link

[DocDB] Fix retryable requests logs to follow standard format #19516

Closed rthallamko3 closed 2 weeks ago

rthallamko3 commented 1 year ago

Jira Link: DB-8309

Description

Some retryable requests logs should have a tablet / peer prefix like all the raft logs

The following logs need to be fixed:

I1013 00:08:27.656975 2504 retryable_requests.cc:258] Initialized RetryableRequestsManager, found a file ? no, wal dir=/mnt/d0/yb-data/tserver/wals/table-878e704957a046dc99f6e5de29ef2fdd/tablet-d4302530b520446ca7b8b923e3b6c4a9

It should follow the convention I1013 00:13:16.665539 2509 tablet_bootstrap.cc:1527] T 8c0741edf81c4e6c941c8ba7e9fc315c P e1b27e86c7354dcd9a17d926ff14d325: Dumping replay state to log at the end of PlaySegments

Issue Type

kind/enhancement

Warning: Please confirm that this issue does not contain any sensitive information

ajd12342 commented 12 months ago

Hi @rthallamko3 ! I am Anuj Diwan, a Computer Science PhD student at UT Austin. I am part of a team along with @arjunrs1 (Arjun Somayazulu) and we're taking a graduate Distributed Systems course. For our course project, we are interested in contributing to Yugabyte. Could we work on this issue? Any pointers for us to get started would be appreciated as well.

Thanks and regards, Anuj.

rthallamko3 commented 12 months ago

@ajd12342 , Refer to Yugabyte contribution page for instructions - https://docs.yugabyte.com/preview/contribute/core-database/checklist/ Feel free to take up this issue. cc @Huqicheng in case you need references to this particular issue. If you get past the initial aspects, we can identify a couple more items in this area.

ajd12342 commented 11 months ago

Hi @Huqicheng @rthallamko3 , thanks, we will take this one up. Please feel free to assign it to us.

A couple of questions:

  1. Do you have a specific list of logging messages that you already know lack tablet or peer ids or should we (automatically/manually) search for them?
  2. Is the convention you mentioned above defined anywhere? I looked into the codebase and it seems that the crux is to use LOG_WITH_PREFIX; is the convention essentially following what LOG_WITH_PREFIX does?
rthallamko3 commented 11 months ago

@ajd12342 , The methods in retryable_requests use the LOG() macro instead of LOG_WITH_PREFIX(). That is correct. Have you checked if the LOG_WITH_PREFIX() for retryable requests prints the tablet id? If not, the logging can use the LogInfo() from the tablet_peer pointer and thus have a unified tracing when compared to messages from tablet_peer.cc. cc @Huqicheng , if he had thought about any other strategy for all the messages in retryable_requests.cc

Huqicheng commented 11 months ago

@ajd12342 In retryable_requests.cc where RetryableRequestsManager is defined, there are several places where LOG_WITH_PREFIX is not used. Please search for LOG(

To use LOG_WITH_PREFIX in RetryableRequestsManager member functions, a field logprefix should be introduced and set from higher layer (e.g. TsTabletManager::OpenTablet or SyscatalogTable::OpenTablet). And also should implement the RetryableRequestsManager::LogPrefix() function to return log_prefix_.

ajd12342 commented 11 months ago

Hi @Huqicheng , we've made a PR that we think fixes this issue. Comments welcome!