pvillard31 commented on code in PR #11372:
URL: https://github.com/apache/nifi/pull/11372#discussion_r3486363739
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java:
##########
@@ -4816,28 +4816,36 @@ public void setStatelessFlowTimeout(final String
statelessFlowTimeout) {
}
private void setLoggingAttributes() {
- loggingAttributes.clear();
+ final Map<String, String> attributes = new HashMap<>();
- loggingAttributes.put(LoggingAttribute.PROCESS_GROUP_ID.attribute, id);
+ attributes.put(LoggingAttribute.PROCESS_GROUP_ID.attribute, id);
final String processGroupName = name.get();
if (processGroupName == null) {
-
loggingAttributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
STANDARD_PROCESS_GROUP_NAME);
+ attributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
STANDARD_PROCESS_GROUP_NAME);
} else {
-
loggingAttributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
processGroupName);
- setGroupPath();
+ attributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
processGroupName);
+ setGroupPath(attributes);
}
final VersionControlInformation currentVersionControl =
versionControlInfo.get();
if (currentVersionControl != null) {
final String registeredFlowIdentifier =
currentVersionControl.getFlowIdentifier();
-
loggingAttributes.put(LoggingAttribute.REGISTERED_FLOW_IDENTIFIER.attribute,
registeredFlowIdentifier);
+
attributes.put(LoggingAttribute.REGISTERED_FLOW_IDENTIFIER.attribute,
registeredFlowIdentifier);
final String registeredFlowVersion =
currentVersionControl.getVersion();
-
loggingAttributes.put(LoggingAttribute.REGISTERED_FLOW_VERSION.attribute,
registeredFlowVersion);
+ attributes.put(LoggingAttribute.REGISTERED_FLOW_VERSION.attribute,
registeredFlowVersion);
}
- loggingAttributes.putAll(connectorLoggingAttributes);
+ attributes.putAll(connectorLoggingAttributes);
+
+ this.loggingAttributes = Map.copyOf(attributes);
+
+ for (final ProcessGroup childGroup : processGroups.values()) {
Review Comment:
The sibling method setConnectorLoggingAttributes iterates getProcessGroups()
(a read-locked copy) while this loop iterates processGroups.values() directly.
Both are safe with a ConcurrentHashMap, but should we use getProcessGroups()
here too for consistency?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java:
##########
@@ -4816,28 +4816,36 @@ public void setStatelessFlowTimeout(final String
statelessFlowTimeout) {
}
private void setLoggingAttributes() {
- loggingAttributes.clear();
+ final Map<String, String> attributes = new HashMap<>();
- loggingAttributes.put(LoggingAttribute.PROCESS_GROUP_ID.attribute, id);
+ attributes.put(LoggingAttribute.PROCESS_GROUP_ID.attribute, id);
final String processGroupName = name.get();
if (processGroupName == null) {
-
loggingAttributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
STANDARD_PROCESS_GROUP_NAME);
+ attributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
STANDARD_PROCESS_GROUP_NAME);
} else {
-
loggingAttributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
processGroupName);
- setGroupPath();
+ attributes.put(LoggingAttribute.PROCESS_GROUP_NAME.attribute,
processGroupName);
+ setGroupPath(attributes);
}
final VersionControlInformation currentVersionControl =
versionControlInfo.get();
if (currentVersionControl != null) {
final String registeredFlowIdentifier =
currentVersionControl.getFlowIdentifier();
-
loggingAttributes.put(LoggingAttribute.REGISTERED_FLOW_IDENTIFIER.attribute,
registeredFlowIdentifier);
+
attributes.put(LoggingAttribute.REGISTERED_FLOW_IDENTIFIER.attribute,
registeredFlowIdentifier);
final String registeredFlowVersion =
currentVersionControl.getVersion();
-
loggingAttributes.put(LoggingAttribute.REGISTERED_FLOW_VERSION.attribute,
registeredFlowVersion);
+ attributes.put(LoggingAttribute.REGISTERED_FLOW_VERSION.attribute,
registeredFlowVersion);
}
- loggingAttributes.putAll(connectorLoggingAttributes);
+ attributes.putAll(connectorLoggingAttributes);
+
+ this.loggingAttributes = Map.copyOf(attributes);
+
+ for (final ProcessGroup childGroup : processGroups.values()) {
+ if (childGroup instanceof StandardProcessGroup standardChildGroup)
{
+ standardChildGroup.setLoggingAttributes();
Review Comment:
Since setConnectorLoggingAttributes already recurses into every child and
each child now also calls setLoggingAttributes(), each descendant gets
recomputed once per ancestor level. Did you consider skipping this subtree
recursion when it is invoked from the connector cascade, to avoid the repeated
recomputation?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
