No-SilverBullet commented on code in PR #3023:
URL: https://github.com/apache/dubbo-go/pull/3023#discussion_r2343159004
##########
filter/accesslog/filter.go:
##########
@@ -84,18 +84,25 @@ func init() {
* AccessLogFilter is designed to be singleton
*/
type Filter struct {
- logChan chan Data
+ logChan chan Data
+ fileCache map[string]*os.File
+ fileLock sync.RWMutex
Review Comment:
put file-lock on fileCache to identify the protected object
##########
filter/accesslog/filter.go:
##########
@@ -182,6 +189,68 @@ func (f *Filter) OnResponse(_ context.Context, result
result.Result, _ base.Invo
return result
}
+// start initializes and starts the background goroutine for processing logs
+func (f *Filter) start() {
+ go f.processLogs()
+}
+
+// processLogs runs in a background goroutine to process log data
+func (f *Filter) processLogs() {
+ defer func() {
+ if r := recover(); r != nil {
+ logger.Errorf("AccessLog processLogs panic: %v", r)
+ }
+ f.drainLogs()
+ }()
+
+ for {
+ select {
+ case accessLogData, ok := <-f.logChan:
+ if !ok {
+ return
+ }
+ f.writeLogToFileWithTimeout(accessLogData,
5*time.Second)
+ case <-f.ctx.Done():
+ return
+ }
+ }
+}
+
+// drainLogs drains remaining log data with timeout protection
+func (f *Filter) drainLogs() {
+ timeout := time.After(5 * time.Second)
+ for {
+ select {
+ case accessLogData, ok := <-f.logChan:
+ if !ok {
+ return
+ }
+ f.writeLogToFileWithTimeout(accessLogData,
1*time.Second)
+ case <-timeout:
+ logger.Warnf("AccessLog drain timeout, some logs may be
lost")
+ return
+ default:
+ return
+ }
+ }
+}
+
+// writeLogToFileWithTimeout writes log with timeout protection
+func (f *Filter) writeLogToFileWithTimeout(data Data, timeout time.Duration) {
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ f.writeLogToFile(data)
+ }()
+
+ select {
+ case <-done:
+ // Success
Review Comment:
maybe we can add a success log here
##########
filter/accesslog/filter.go:
##########
@@ -204,12 +273,52 @@ func (f *Filter) writeLogToFile(data Data) {
}
}
+// getOrOpenLogFile gets or opens the log file with proper caching and handle
management
+func (f *Filter) getOrOpenLogFile(accessLog string) (*os.File, error) {
Review Comment:
Is the reason for judging twice here to reduce the time of writing lock?
##########
filter/accesslog/filter.go:
##########
@@ -204,12 +273,52 @@ func (f *Filter) writeLogToFile(data Data) {
}
}
+// getOrOpenLogFile gets or opens the log file with proper caching and handle
management
+func (f *Filter) getOrOpenLogFile(accessLog string) (*os.File, error) {
+ f.fileLock.RLock()
+ if logFile, exists := f.fileCache[accessLog]; exists {
+ // Check if we need to rotate the log
+ now := time.Now().Format(FileDateFormat)
+ if fileInfo, err := logFile.Stat(); err == nil {
+ last := fileInfo.ModTime().Format(FileDateFormat)
+ if now == last {
+ f.fileLock.RUnlock()
+ return logFile, nil
+ }
Review Comment:
this logic can be extract to a func
##########
filter/accesslog/filter.go:
##########
@@ -204,12 +273,52 @@ func (f *Filter) writeLogToFile(data Data) {
}
}
+// getOrOpenLogFile gets or opens the log file with proper caching and handle
management
+func (f *Filter) getOrOpenLogFile(accessLog string) (*os.File, error) {
+ f.fileLock.RLock()
+ if logFile, exists := f.fileCache[accessLog]; exists {
+ // Check if we need to rotate the log
+ now := time.Now().Format(FileDateFormat)
+ if fileInfo, err := logFile.Stat(); err == nil {
+ last := fileInfo.ModTime().Format(FileDateFormat)
+ if now == last {
+ f.fileLock.RUnlock()
+ return logFile, nil
+ }
+ }
+ }
+ f.fileLock.RUnlock()
+
+ // Need to open new file or rotate existing one
+ f.fileLock.Lock()
+ defer f.fileLock.Unlock()
+
+ // Double-check after acquiring write lock
+ if logFile, exists := f.fileCache[accessLog]; exists {
+ now := time.Now().Format(FileDateFormat)
+ if fileInfo, err := logFile.Stat(); err == nil {
+ last := fileInfo.ModTime().Format(FileDateFormat)
+ if now == last {
+ return logFile, nil
+ }
+ }
+ // Close the old file before rotation
+ logFile.Close()
Review Comment:
check error
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
