Backport of memory settings fixups from https://github.com/TouK/nussk… #7279
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant updates to the Nussknacker application, as documented in the changelog and configuration files. Key enhancements include the addition of an Activities panel, which consolidates scenario activities, and the introduction of scenario labels for better organization. Improvements to the Spring Expression Language (SpEL) are also noted, including enhanced navigation for unknown variable types and new methods for data type conversion, JSON handling, and BASE64 operations. A major upgrade to Flink version 1.19.1 is included, along with performance optimizations for event serialization. Modifications to the Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
examples/installation/docker-compose.yml (1)
43-43: Consider adding memory monitoring and tuning guidelinesWith the memory settings now more granular across services, consider:
- Adding monitoring dashboards for memory usage
- Documenting the process for tuning these values
- Creating alerts for memory pressure
- Adding comments explaining the memory calculations
This will help operators maintain and tune the system effectively.
Also applies to: 244-244
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
docs/Changelog.md(1 hunks)examples/installation/docker-compose.yml(2 hunks)examples/installation/flink/flink-properties.yml(1 hunks)
🔇 Additional comments (4)
docs/Changelog.md (1)
28-28: LGTM!
The changelog entry is clear, properly formatted, and correctly references the PR number.
examples/installation/flink/flink-properties.yml (1)
2-4: Verify memory allocation ratios
The taskmanager memory configuration allocates:
- 1500MB total process size
- 400MB for metaspace (26.7% of total)
This leaves approximately 1100MB for heap and other memory areas, which might be tight depending on the workload. Consider documenting the memory breakdown in the comment to justify these specific values.
Let's verify if these memory settings are consistent with recommended Flink memory configurations:
✅ Verification successful
Let me gather more context about the memory configuration by looking for any documentation or tests that might justify these values.
Memory configuration aligns with documented recommendations
The memory settings are justified based on the documentation and test configurations found:
- The operations guide explicitly states that "Metaspace size should be fairly large both on jobmanagers and taskmanagers - for typical deployments with around 20-30 scenarios value around 1 GB should be enough"
- The current configuration of 400MB metaspace is actually conservative compared to:
- The recommended 1GB for typical deployments
- The 512MB used in test configurations
- The documentation warns about metaspace-related issues and classloading, suggesting that generous metaspace allocation is important for stability
The current configuration represents a balanced approach, allocating sufficient metaspace while leaving adequate memory for other operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any memory-related configuration documentation or examples
rg -A 3 "taskmanager.memory" --type yaml
rg -A 3 "metaspace" --type yaml
Length of output: 1885
Script:
#!/bin/bash
# Search for documentation about memory configuration
rg -g "*.md" -i "memory|metaspace" docs/
rg -g "README*" -i "memory|metaspace"
# Look for any tests that might validate memory settings
rg -g "*.{java,scala}" -i "memory.*test|metaspace.*test" engine/flink/
Length of output: 4559
examples/installation/docker-compose.yml (2)
244-244: Verify Flink TaskManager memory limit consistency
The memory limit (1500M) matches the taskmanager.memory.process.size in flink-properties.yml, which is good. However, we should ensure this increase from 1024M to 1500M is sufficient for the workload.
Let's verify the memory requirements across the configuration:
✅ Verification successful
Memory configuration appears consistent and sufficient
The memory configuration appears well-aligned across the system:
- The TaskManager memory limit (1500M) in docker-compose.yml matches exactly with
taskmanager.memory.process.size: 1500min flink-properties.yml - The configuration includes a generous metaspace allocation of 400m
- This memory allocation is higher than what's used in the development/testing environments (1024M)
- The benchmarks environment uses 2048M, suggesting 1500M is a reasonable middle ground
- RocksDB managed memory is explicitly disabled, reducing memory pressure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for memory-related configurations and requirements
rg "memory" --type yaml
rg "heap|metaspace" --type yaml
Length of output: 3234
43-43: Review memory settings breakdown for Designer service
The JVM memory settings are now more granular:
- Heap: 400MB
- Metaspace: 300MB
- Direct Memory: 100MB
Total: 800MB out of 1024MB container limit
This provides better memory isolation but leaves only 224MB for the container OS and other overhead. Consider documenting the reasoning behind these specific values.
Let's verify if these memory settings are sufficient based on historical usage:
✅ Verification successful
Based on the search results, I can make a final assessment:
Memory settings for Designer service appear appropriate
The current memory settings (400MB heap, 300MB metaspace, 100MB direct) are well-justified based on the codebase evidence:
- Historical memory settings in other environments were similar or lower:
- docker-compose.yml in designer/client:
-Xmx512M - No previous metaspace/direct memory issues reported
- docker-compose.yml in designer/client:
- Changelog shows only two memory-related fixes ([NU-1609] fixed: Nu runtime memory leak #5898, [NU-1683] Fixes memory leak in test mechanism introduced in #4956 #6337) which were specific implementation leaks, not configuration issues
- The 224MB overhead for container OS is sufficient as Designer is primarily a web application without heavy processing needs
The memory breakdown provides better isolation and matches the service's requirements. No changes needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any previous memory-related issues or discussions
rg -A 3 "OutOfMemoryError|memory" --type md
rg -A 3 "Xmx|MaxMetaspaceSize|MaxDirectMemorySize" --type yaml
Length of output: 8452
…nacker-quickstart/pull/197 and https://github.com/TouK/nussknacker-quickstart/pull/198/files to installation example
Describe your changes
Checklist before merge
Summary by CodeRabbit
Release Notes
New Features
Performance Improvements
Bug Fixes
Configuration Updates
designerandflink-taskmanagerservices to enhance performance.