-
Notifications
You must be signed in to change notification settings - Fork 469
feat: Log flag optimizations (profile/machine-type) in mount logs #3965
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
base: master
Are you sure you want to change the base?
feat: Log flag optimizations (profile/machine-type) in mount logs #3965
Conversation
|
/gemini summary |
|
/gemini review |
|
/gemini summary |
|
/gemini review |
Summary of ChangesThis pull request introduces a significant improvement to GCSFuse's logging by capturing and reporting details about configuration flags that are automatically optimized based on factors like machine type or predefined profiles. This change moves from simply listing optimized flags to providing a structured map that includes the final value, the reason for optimization, and whether it was optimized. This enhanced logging will greatly assist in debugging, understanding, and verifying the configuration applied during GCSFuse mounts. Highlights
Changelog
Activity
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request enhances the logging of flag optimizations by changing the ApplyOptimizations function to return a map with detailed optimization results. This provides better insight into why a flag's value was changed. The implementation is sound and correctly propagated through the configuration generation, application logic, and tests. I have a couple of minor suggestions to improve code style and performance.
|
Can you test it once how does these optimisation flags look in the GKE Log Explorer? |
Yes, I shared a sample generated log in the PR description in manual testing section. Copying here... # preexisting log line(s)
{"timestamp":{"seconds":1762273085,"nanos":636732946},"severity":"INFO","message":"Start gcsfuse/unknown (Go version go1.24.8) for app \"\" using mount point: gargnitin-test-hns-asiase1-mount\n","mount-id":"gargnitin-test-hns-asiase1-e7164ab0"}
{"timestamp":{"seconds":1762273085,"nanos":636767706},"severity":"INFO","message":"GCSFuse Config","mount-id":"gargnitin-test-hns-asiase1-e7164ab0","CLI Flags":{"log-format":"json","machine-type":"a2-megagpu-16g","profile":"aiml-training"}}
# New log line(s)
{"timestamp":{"seconds":1762273085,"nanos":636786216},"severity":"INFO","message":"GCSFuse Config","mount-id":"gargnitin-test-hns-asiase1-e7164ab0","Flags optimized":{"file-system.rename-dir-limit":{"FinalValue":200000,"OptimizationReason":"machine-type group \"high-performance\"","Optimized":true},"implicit-dirs":{"FinalValue":true,"OptimizationReason":"profile \"aiml-training\"","Optimized":true},"metadata-cache.negative-ttl-secs":{"FinalValue":0,"OptimizationReason":"profile \"aiml-training\"","Optimized":true},"metadata-cache.stat-cache-max-size-mb":{"FinalValue":-1,"OptimizationReason":"profile \"aiml-training\"","Optimized":true},"metadata-cache.ttl-secs":{"FinalValue":-1,"OptimizationReason":"profile \"aiml-training\"","Optimized":true},"metadata-cache.type-cache-max-size-mb":{"FinalValue":-1,"OptimizationReason":"profile \"aiml-training\"","Optimized":true},"write.global-max-blocks":{"FinalValue":1600,"OptimizationReason":"machine-type group \"high-performance\"","Optimized":true}}}
# preexisting log line(s)
{"timestamp":{"seconds":1762273085,"nanos":636827356},"severity":"INFO","message":"GCSFuse Config","mount-id":"gargnitin-test-hns-asiase1-e7164ab0","Full Config":{"AppName":"","CacheDir":"","CloudProfiler":{"AllocatedHeap":true,"Cpu":true,"Enabled":false,"Goroutines":false,"Heap":true,"Label":"gcsfuse-0.0.0","Mutex":false},"Debug":{"ExitOnInvariantViolation":false,"Fuse":false,"Gcs":false,"LogMutex":false},"DisableAutoconfig":false,"EnableAtomicRenameObject":true,"EnableGoogleLibAuth":true,"EnableHns":true,"EnableNewReader":true,"EnableUnsupportedDirSupport":false,"FileCache":{"CacheFileForRangeRead":false,"DownloadChunkSizeMb":200,"EnableCrc":false,"EnableODirect":false,"EnableParallelDownloads":false,"ExcludeRegex":"","ExperimentalParallelDownloadsDefaultOn":true,"IncludeRegex":"","MaxParallelDownloads":128,"MaxSizeMb":-1,"ParallelDownloadsPerFile":16,"WriteBufferSize":4194304},"FileSystem":{"DirMode":"755","DisableParallelDirops":false,"ExperimentalEnableDentryCache":false,"ExperimentalEnableReaddirplus":false,"FileMode":"644","FuseOptions":[],"Gid":-1,"IgnoreInterrupts":true,"KernelListCacheTtlSecs":0,"ODirect":false,"PreconditionErrors":true,"RenameDirLimit":200000,"TempDir":"","Uid":-1},"Foreground":false,"GcsAuth":{"AnonymousAccess":false,"KeyFile":"","ReuseTokenFromUrl":true,"TokenUrl":""},"GcsConnection":{"BillingProject":"","ClientProtocol":"http1","CustomEndpoint":"","EnableHttpDnsCache":true,"ExperimentalEnableJsonRead":false,"GrpcConnPoolSize":1,"HttpClientTimeout":0,"LimitBytesPerSec":-1,"LimitOpsPerSec":-1,"MaxConnsPerHost":0,"MaxIdleConnsPerHost":100,"SequentialReadSizeMb":200},"GcsRetries":{"ChunkTransferTimeoutSecs":10,"MaxRetryAttempts":0,"MaxRetrySleep":30000000000,"Multiplier":2,"ReadStall":{"Enable":true,"InitialReqTimeout":20000000000,"MaxReqTimeout":1200000000000,"MinReqTimeout":1500000000,"ReqIncreaseRate":15,"ReqTargetPercentile":0.99}},"ImplicitDirs":true,"List":{"EnableEmptyManagedFolders":false},"Logging":{"FilePath":"","Format":"json","LogRotate":{"BackupFileCount":10,"Compress":true,"MaxFileSizeMb":512},"Severity":"INFO"},"MachineType":"a2-megagpu-16g","MetadataCache":{"DeprecatedStatCacheCapacity":20460,"DeprecatedStatCacheTtl":60000000000,"DeprecatedTypeCacheTtl":60000000000,"EnableNonexistentTypeCache":false,"ExperimentalMetadataPrefetchOnMount":"disabled","NegativeTtlSecs":0,"StatCacheMaxSizeMb":17592186044415,"TtlSecs":9223372036,"TypeCacheMaxSizeMb":-1},"Metrics":{"BufferSize":256,"CloudMetricsExportIntervalSecs":0,"PrometheusPort":0,"StackdriverExportInterval":0,"UseNewNames":false,"Workers":3},"Monitoring":{"ExperimentalTracingMode":"","ExperimentalTracingProjectId":"","ExperimentalTracingSamplingRatio":0},"OnlyDir":"","Profile":"aiml-training","Read":{"BlockSizeMb":16,"EnableBufferedRead":false,"GlobalMaxBlocks":40,"InactiveStreamTimeout":10000000000,"MaxBlocksPerHandle":20,"MinBlocksPerHandle":4,"RandomSeekThreshold":3,"StartBlocksPerHandle":1},"WorkloadInsight":{"OutputFile":"","Visualize":false},"Write":{"BlockSizeMb":32,"CreateEmptyFile":false,"EnableRapidAppends":true,"EnableStreamingWrites":true,"FinalizeFileForRapid":false,"GlobalMaxBlocks":1600,"MaxBlocksPerFile":1}}}
{"timestamp":{"seconds":1762273085,"nanos":931233199},"severity":"INFO","message":"File system has been successfully mounted.","mount-id":"gargnitin-test-hns-asiase1-e7164ab0"} |
3364d8a to
e2230d6
Compare
I was asking about how it gets parsed in the LogExplorer so that we can be sure all of the fields are correctly displaying in the json format in the LogExplorer. |
ok, I'll test it in gke and share. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3965 +/- ##
==========================================
+ Coverage 83.05% 83.06% +0.01%
==========================================
Files 148 148
Lines 18149 18185 +36
==========================================
+ Hits 15074 15106 +32
- Misses 2523 2527 +4
Partials 552 552
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sample log: https://cloudlogging.app.goo.gl/Rj5AY9UM7ANfAn8FA
|
|
Is the |
|
Also why do we need |
|
@meet2mky I've updated the logs to both hide the Latest log: https://cloudlogging.app.goo.gl/AapxGw2qMHL7uzZ99 |
|
/gemini review |
|
/gemini summary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request significantly improves the logging of optimized configuration flags by providing a structured map with detailed information. The changes are well-implemented, and the addition of tests for the new hierarchical map creation logic is appreciated. My main feedback is to address code duplication by centralizing the createHierarchicalOptimizedFlags function.
Summary of ChangesThis pull request significantly enhances GCSFuse's logging capabilities by providing detailed, structured information about configuration flags that are automatically optimized based on machine type or predefined profiles. Instead of a simple list, the logs now include the final value, the reason for optimization, and whether a flag was optimized, greatly aiding in debugging and verifying GCSFuse configurations. Highlights
Changelog
Activity
|
|
Output is looking great 💯 , do we already have this method in cfg package? please review the gemini comment. |
No, it was a unintentional dup because of a miss in refactor. Fixed it now. |
meet2mky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few minor comments.
ae035c0 to
4703c81
Compare
meet2mky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
remove unused code
3baad76 to
c404b90
Compare

Description
Log flag optimizations (profile/machine-type) in mount logs
This pull request introduces a significant improvement to GCSFuse's logging by capturing and reporting details about configuration flags that are automatically optimized based on factors like machine type or predefined profiles. This change moves from simply listing optimized flags to providing a structured map that includes the final value, the reason for optimization, and whether it was optimized. This enhanced logging will greatly assist in debugging, understanding, and verifying the configuration applied during GCSFuse mounts.
Highlights
ApplyOptimizationsfunction incfg/config.goand its template now returns amap[string]OptimizationResultinstead of a[]string, allowing for richer data about each optimized flag.optimizedFlags(typemap[string]cfg.OptimizationResult), has been added to themountInfostruct incmd/root.goto store the detailed results of flag optimizations.isFlagPresentInOptimizationResults, was introduced incfg/optimize_test.goto facilitate testing the presence of specific flags within the newoptimizedFlagsmap.cmd/legacy_main.gofile has been updated to include theoptimizedFlagsin the GCSFuse mount logs, making this information readily available during operation.Link to the issue in case of a bug fix.
b/456466737
Testing details
{"timestamp":{"seconds":1762415296,"nanos":154084615},"severity":"INFO","message":"GCSFuse Config","mount-id":"gargnitin-test-hns-asiase1-9fcf07c7","Optimized Flags":{"file-system":{"rename-dir-limit":{"final_value":200000,"optimization_reason":"machine-type group \"high-performance\""}},"implicit-dirs":{"final_value":true,"optimization_reason":"profile \"aiml-training\""},"metadata-cache":{"negative-ttl-secs":{"final_value":0,"optimization_reason":"profile \"aiml-training\""},"stat-cache-max-size-mb":{"final_value":-1,"optimization_reason":"profile \"aiml-training\""},"ttl-secs":{"final_value":-1,"optimization_reason":"profile \"aiml-training\""},"type-cache-max-size-mb":{"final_value":-1,"optimization_reason":"profile \"aiml-training\""}},"write":{"global-max-blocks":{"final_value":1600,"optimization_reason":"machine-type group \"high-performance\""}}}}Any backward incompatible change? If so, please explain.