Skip to content

[AMBARI-22878] Update Service Group API to take list of mpack name associated with the service group #234

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.

Sign up for GitHub

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

Merged
jayush merged 17 commits into from
Feb 22, 2018
Merged

[AMBARI-22878] Update Service Group API to take list of mpack name associated with the service group #234

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
dcb871b
AMBARI-22878: Update Service Group API to take list of mpack name ass…
scottduan Jan 30, 2018
e97eb29
AMBARI-22878: Update Service Group API to take list of mpack name ass…
scottduan Jan 30, 2018
da41ca0
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 1, 2018
158cd85
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 1, 2018
decbf54
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 5, 2018
68c1f04
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
a2340a6
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
04345f8
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
scottduan Feb 9, 2018
90f354f
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
a331735
Merge remote-tracking branch 'upstream/branch-feature-AMBARI-14714' i…
scottduan Feb 9, 2018
3de0e93
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 16, 2018
f74af11
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 16, 2018
afc6a3b
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 18, 2018
fb3e005
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 18, 2018
7bcd6fe
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 22, 2018
cfe5295
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 22, 2018
d370602
Merge branch 'AMBARI-22878-branch-feature-AMBARI-14714' of https://gi…
Feb 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceGroupRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@
*/
package org.apache.ambari.server.controller;

import java.util.HashSet;
import java.util.Objects;
import java.util.Set;

public class ServiceGroupRequest {

private String clusterName; // REF
private String serviceGroupName; // GET/CREATE/UPDATE/DELETE
private Set<String> mpackNames; // Associated mpack names

public ServiceGroupRequest(String clusterName, String serviceGroupName) {
this.clusterName = clusterName;

@jonathan-hurley jonathan-hurley Feb 20, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's still a misunderstanding here - I thought it would be cleaner to separate this out into 2 fields so that it's clear to the user what is expected: mpack name and then mpack version. You can combine this and call it a stack ID internally, that's fine - but we should take 2 different properties.

Same goes for API requests where we're currently given name-version as a string. This should be broken out into 2 fields.

@scottduan scottduan Feb 21, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok for this change.

@scottduan scottduan Feb 21, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayush What do you think?

@jayush jayush Feb 21, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep it as "version" for now for legacy purposes. When we upgrade to ambari 3.0 assuming the existing cluster is on HDP-2.6 stack, calling this as mpack name and mpack version would be misleading.

We should handle this in a separate JIRA if we really want to change this convention for all APIs.

@jonathan-hurley jonathan-hurley Feb 22, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But "version" in legacy terms meant only the numeric value. If we don't want to break it out into 2 references, I think we'd rather use stackId, right?

this.serviceGroupName = serviceGroupName;
mpackNames = new HashSet<>();
}

/**
Expand Down Expand Up @@ -57,9 +61,30 @@ public void setServiceGroupName(String serviceGroupName) {
this.serviceGroupName = serviceGroupName;
}

/**
* @return a list of associated mpack names
*/
public Set<String> getMpackNames() {
return mpackNames;
}

/**
* @param mpackNames a list of associated mpack names
*/
public void addMpackNames(Set<String> mpackNames) {
if (mpackNames != null) {
this.mpackNames.addAll(mpackNames);
}
}

@Override
public String toString() {
return String.format("clusterName=%s, serviceGroupName=%s", clusterName, serviceGroupName);
StringBuilder sb = new StringBuilder();

@ncole ncole Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been gradually moving to ToStringBuilder here instead

@jonathan-hurley jonathan-hurley Feb 16, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should switch this to ToStringBuilder.

sb.append("clusterName=").append(clusterName).append(", serviceGroupName=").append(serviceGroupName);
if (!mpackNames.isEmpty()) {
sb.append(",mpackNames=").append(mpackNames.toString());
}
return sb.toString();
}

@Override
Expand All @@ -73,6 +98,8 @@ public boolean equals(Object obj) {

ServiceGroupRequest other = (ServiceGroupRequest) obj;

// ignore mpackNames, even if they are different, we still consider sgrequests are the same

return Objects.equals(clusterName, other.clusterName) &&
Objects.equals(serviceGroupName, other.serviceGroupName);
}
Expand Down
23 changes: 23 additions & 0 deletions ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceGroupResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

package org.apache.ambari.server.controller;

import java.util.HashSet;
import java.util.Set;

import io.swagger.annotations.ApiModelProperty;

public class ServiceGroupResponse {
Expand All @@ -26,12 +29,15 @@ public class ServiceGroupResponse {
private Long serviceGroupId;
private String clusterName;
private String serviceGroupName;
private Set<String> mpackNames;

public ServiceGroupResponse(Long clusterId, String clusterName, Long serviceGroupId, String serviceGroupName) {
this.clusterId = clusterId;
this.serviceGroupId = serviceGroupId;
this.clusterName = clusterName;
this.serviceGroupName = serviceGroupName;
this.mpackNames = new HashSet<>();

}

/**
Expand Down Expand Up @@ -90,6 +96,22 @@ public void setServiceGroupName(String serviceGroupName) {
this.serviceGroupName = serviceGroupName;
}

/**
* @return the list of mpack names
*/
public Set<String> getMpackNames() {
return mpackNames;
}

/**
* @param mpackNames the list of mpack names
*/
public void setMpackNames(Set<String> mpackNames) {
if (mpackNames != null) {
this.mpackNames = mpackNames;
}
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand All @@ -109,6 +131,7 @@ public boolean equals(Object o) {
!serviceGroupName.equals(that.serviceGroupName) : that.serviceGroupName != null) {
return false;
}
// ignore mpackNames

return true;
}
Expand Down
19 changes: 19 additions & 0 deletions .../main/java/org/apache/ambari/server/controller/internal/ServiceGroupResourceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
package org.apache.ambari.server.controller.internal;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import org.apache.ambari.server.AmbariException;
import org.apache.ambari.server.ClusterNotFoundException;
Expand Down Expand Up @@ -78,6 +80,7 @@ public class ServiceGroupResourceProvider extends AbstractControllerResourceProv
public static final String SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "cluster_name";
public static final String SERVICE_GROUP_SERVICE_GROUP_ID_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "service_group_id";
public static final String SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "service_group_name";
public static final String SERVICE_GROUP_MPACKNAME_PROPERTY_ID = RESPONSE_KEY + PropertyHelper.EXTERNAL_PATH_SEP + "mpacks";

@ncole ncole Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an awkward construct. Either do the string directly or use PropertyHelper.getPropertyId(...).

@adoroszlai adoroszlai Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ncole This is useful for when we document the API using Swagger annotations. The compiler does not allow using strings generated by getPropertyId() method call in annotation parameters. Using the string directly in multiple places is prone to typos. Hence this awkward construct was born.

Anyway, this question does not have much to do with this PR, as @scottduan just followed the existing usage pattern in the lines above.

@scottduan scottduan Feb 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mpack:sg is many to many relationship finally. At this time, we just have temporary restriction for 1:1, also I did not validate mpack information. This will be done in the following mpack work.

@jonathan-hurley jonathan-hurley Feb 6, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it should be M2M - it should be M2O. Since upgrade workflow is based off of SGs, it doesn't make sense to have the ability to mix them at this point. It creates more complexity that will not be used.

Also, this relationship shouldn't be based on names, but on specific IDs of the mpack. There shouldn't be ambiguity about which MPack a SG is based off of.

@ncole ncole Feb 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adoroszlai - it's a constant, spelling should not be an issue. You can't use constants in Swagger? That would be extremely disappointing.

@scottduan scottduan Feb 6, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayush , how do you think? Based on my understand, SG can pick up service from different mpacks, so this should be M2M.
@jonathan-hurley , because we do not consider version. If we use mpack id, that means we also consider mpack version. As far as I know, now we only consider mpack name, not including version.

@jonathan-hurley jonathan-hurley Feb 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A SG should not be able to pickup services from different mpacks - it will break a lot of upgrade flow and assumptions. Additionally, it needs to be associated with a specific mpack in order to provide a way of querying what SG's are compatible with given mpack.

@scottduan scottduan Feb 6, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mpack provides with service dependence list. So when SG picks up service, theoratically it can choose proper service with the right version (it should be a range). There has a mpack design doc, https://docs.google.com/document/d/13y2T_uN8_nrMsNRuqXLnQAm64Edie0k8_WVNOFvKq0U/edit#heading=h.ykmhjj9wbfa8
We may need to schedule a meeting to clarify how to implement mpack feature in HDP3.

@adoroszlai adoroszlai Feb 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ncole It's a limitation of annotation "element value" in Java, not specific to Swagger. The value must be a constant expression, not just any runtime constant.



private static Set<String> pkPropertyIds =
Expand All @@ -103,10 +106,12 @@ public class ServiceGroupResourceProvider extends AbstractControllerResourceProv
PROPERTY_IDS.add(SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID);
PROPERTY_IDS.add(SERVICE_GROUP_SERVICE_GROUP_ID_PROPERTY_ID);
PROPERTY_IDS.add(SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID);
PROPERTY_IDS.add(SERVICE_GROUP_MPACKNAME_PROPERTY_ID);

// keys
KEY_PROPERTY_IDS.put(Resource.Type.Cluster, SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID);
KEY_PROPERTY_IDS.put(Resource.Type.ServiceGroup, SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID);
KEY_PROPERTY_IDS.put(Resource.Type.Mpack, SERVICE_GROUP_MPACKNAME_PROPERTY_ID);
}

private Clusters clusters;
Expand Down Expand Up @@ -160,6 +165,7 @@ public Set<ServiceGroupResponse> invoke() throws AmbariException, AuthorizationE
resource.setProperty(SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID, response.getClusterName());
resource.setProperty(SERVICE_GROUP_SERVICE_GROUP_ID_PROPERTY_ID, response.getServiceGroupId());
resource.setProperty(SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID, response.getServiceGroupName());
resource.setProperty(SERVICE_GROUP_MPACKNAME_PROPERTY_ID, response.getMpackNames());

associatedResources.add(resource);
}
Expand Down Expand Up @@ -199,6 +205,8 @@ public Set<ServiceGroupResponse> invoke() throws AmbariException {
response.getServiceGroupId(), requestedIds);
setResourceProperty(resource, SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID,
response.getServiceGroupName(), requestedIds);
setResourceProperty(resource, SERVICE_GROUP_MPACKNAME_PROPERTY_ID,
response.getMpackNames(), requestedIds);
resources.add(resource);
}
return resources;
Expand Down Expand Up @@ -269,6 +277,11 @@ private ServiceGroupRequest getRequest(Map<String, Object> properties) {
String clusterName = (String) properties.get(SERVICE_GROUP_CLUSTER_NAME_PROPERTY_ID);
String serviceGroupName = (String) properties.get(SERVICE_GROUP_SERVICE_GROUP_NAME_PROPERTY_ID);
ServiceGroupRequest svcRequest = new ServiceGroupRequest(clusterName, serviceGroupName);
Collection<Map<String,String>> mpackNames = (Collection<Map<String,String>>) properties.get(SERVICE_GROUP_MPACKNAME_PROPERTY_ID);
if (mpackNames != null) {
Set<String> mpackNamesSet = mpackNames.stream().flatMap(mpack -> mpack.values().stream()).collect(Collectors.toSet());
svcRequest.addMpackNames(mpackNamesSet);
}
return svcRequest;
}

Expand All @@ -292,6 +305,7 @@ public synchronized Set<ServiceGroupResponse> createServiceGroups(Set<ServiceGro

// Already checked that service group does not exist
ServiceGroup sg = cluster.addServiceGroup(request.getServiceGroupName());
sg.setMpackNames(request.getMpackNames());
createdSvcGrps.add(sg.convertToResponse());
}
return createdSvcGrps;
Expand Down Expand Up @@ -422,6 +436,11 @@ private void validateCreateRequests(Set<ServiceGroupRequest> requests, Clusters
}
serviceGroupNames.get(clusterName).add(serviceGroupName);

if (request.getMpackNames().size() != 1) {

@adoroszlai adoroszlai Feb 2, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this change introduces mpacks for service groups, and it also makes exactly one mpack a requirement, it breaks existing code that uses ServiceGroupResourceProvider to create mpacks. For one, it increases the number of failing/erroring unit tests:

[ERROR] Tests run: 5122, Failures: 39, Errors: 231, Skipped: 37
->
[ERROR] Tests run: 5123, Failures: 42, Errors: 296, Skipped: 37

Can you please address those? I think most could be fixed by adding some dummy mpack name in

ambari/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceGroupResourceProviderTest.java

Lines 39 to 43 in 00f29b9

public static void createServiceGroup(AmbariManagementController controller, String clusterName, String serviceGroupName)
throws AmbariException, AuthorizationException {
ServiceGroupRequest request = new ServiceGroupRequest(clusterName, serviceGroupName);
createServiceGroups(controller, Collections.singleton(request));
}

String errmsg = "Invalid arguments, " + request.getMpackNames().size() + " mpack(s) found in the service group " + serviceGroupName + ", only one mpack is allowed";

@ncole ncole Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use String.format() here.

@scottduan scottduan Feb 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that concatenation has better performance than string.format().

@ncole ncole Feb 6, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For messages like this, we're more interested in readability in code than performance. We've been using String.format() in more and more places to be a bit more consistent with logging syntax (even if they are not identical).

@scottduan scottduan Feb 6, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will update it.

@scottduan scottduan Feb 6, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I use StringBuilder instead? I saw your another comment mentioned it.

throw new IllegalArgumentException(errmsg);
}

Cluster cluster;
try {
cluster = clusters.getCluster(clusterName);
Expand Down
22 changes: 22 additions & 0 deletions ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceGroupEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@

package org.apache.ambari.server.orm.entities;

import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.persistence.CollectionTable;
import javax.persistence.Column;
import javax.persistence.ElementCollection;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
Expand Down Expand Up @@ -80,6 +84,14 @@ public class ServiceGroupEntity {
@OneToMany(mappedBy="serviceGroupDependency")
private List<ServiceGroupDependencyEntity> dependencies;

@ElementCollection()

@ncole ncole Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting

@scottduan scottduan Feb 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a little more there? Format what? Thanks.

@ncole ncole Feb 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have meant the next lines down - seems like a lot of whitespace.

@scottduan scottduan Feb 6, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove all those whitespace, and put them on the same line.

@CollectionTable(name = "servicegroup_mpacknames",
joinColumns = {@JoinColumn(name = "service_group_id", referencedColumnName = "id"),
@JoinColumn(name = "service_group_cluster_id", referencedColumnName = "cluster_id")})
@Column(name = "mpack_name", unique = true, nullable = false)

@adoroszlai adoroszlai Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the mpack_name in itself should be unique. This causes unit test failures with the current constant "dummy" mpack name for tests. (eg. AmbariManagementControllerTest)

@scottduan scottduan Feb 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually mpack_name should not be unique. It should allow different service group to have the same mpack_name. I will remove this contraint.


private Set<String> mpackNames = new HashSet<>();

public Long getClusterId() {
return clusterId;
}
Expand Down Expand Up @@ -121,6 +133,16 @@ public void setServiceGroupDependencies(List<ServiceGroupDependencyEntity> servi
this.serviceGroupDependencies = serviceGroupDependencies;
}

public Set<String> getMpackNames() {
return mpackNames;
}

public void setMpackNames(Set<String> mpackNames) {
if (mpackNames != null) {
this.mpackNames = mpackNames;
}
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down
8 changes: 8 additions & 0 deletions ambari-server/src/main/java/org/apache/ambari/server/state/ServiceGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ public interface ServiceGroup {

Set<ServiceGroupDependencyResponse> getServiceGroupDependencyResponses();

Set<String> getMpackNames();

void addMpackName(String mpackName);

void addMpackNames(Set<String> mpackNames);

void setMpackNames(Set<String> mpackNames);

void debugDump(StringBuilder sb);

void refresh();
Expand Down
46 changes: 40 additions & 6 deletions ambari-server/src/main/java/org/apache/ambari/server/state/ServiceGroupImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,36 @@ public long getClusterId() {
return cluster.getClusterId();
}

@Override
public Set<String> getMpackNames() {
ServiceGroupEntity entity = getServiceGroupEntity();
return entity.getMpackNames();
}

@Override
public void addMpackName(String mpackName){
ServiceGroupEntity entity = getServiceGroupEntity();
if (entity.getMpackNames().add(mpackName)) {
serviceGroupDAO.merge(entity);
}
}

@Override
public void addMpackNames(Set<String> mpackNames) {
if (mpackNames != null) {
ServiceGroupEntity entity = getServiceGroupEntity();
entity.getMpackNames().addAll(mpackNames);
serviceGroupDAO.merge(entity);
}
}

@Override
public void setMpackNames(Set<String> mpackNames) {
ServiceGroupEntity entity = getServiceGroupEntity();
entity.getMpackNames().addAll(mpackNames);
serviceGroupDAO.merge(entity);
}

@Override
public Set<ServiceGroupKey> getServiceGroupDependencies() {
return serviceGroupDependencies;
Expand All @@ -149,6 +179,7 @@ public void setServiceGroupDependencies(Set<ServiceGroupKey> serviceGroupDepende
public ServiceGroupResponse convertToResponse() {
ServiceGroupResponse r = new ServiceGroupResponse(cluster.getClusterId(),
cluster.getClusterName(), getServiceGroupId(), getServiceGroupName());
r.setMpackNames(getMpackNames());
return r;
}

Expand Down Expand Up @@ -216,8 +247,12 @@ public Cluster getCluster() {

@Override
public void debugDump(StringBuilder sb) {
sb.append("ServiceGroup={ serviceGroupName=" + getServiceGroupName() + ", clusterName="
+ cluster.getClusterName() + ", clusterId=" + cluster.getClusterId() + "}");
sb.append("ServiceGroup={ serviceGroupName=").append(getServiceGroupName()).append(", clusterName=").append(cluster.getClusterName()).append(", clusterId=").append(cluster.getClusterId());
Set<String> mpackNames = getMpackNames();
if (!mpackNames.isEmpty()) {
sb.append(", mpackNames=").append(mpackNames.toString());
}
sb.append("}");
}

/**
Expand Down Expand Up @@ -254,10 +289,9 @@ protected ServiceGroupEntity persistEntities(ServiceGroupEntity serviceGroupEnti
@Override
@Transactional
public void refresh() {
ServiceGroupEntityPK pk = new ServiceGroupEntityPK();
pk.setClusterId(getClusterId());
pk.setServiceGroupId(getServiceGroupId());
ServiceGroupEntity serviceGroupEntity = serviceGroupDAO.findByPK(pk);
serviceGroupEntityPK.setClusterId(getClusterId());
serviceGroupEntityPK.setServiceGroupId(getServiceGroupId());
ServiceGroupEntity serviceGroupEntity = serviceGroupDAO.findByPK(serviceGroupEntityPK);
serviceGroupDAO.refresh(serviceGroupEntity);
}

Expand Down
10 changes: 10 additions & 0 deletions ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,16 @@ CREATE TABLE alert_notice (
FOREIGN KEY (history_id) REFERENCES alert_history(alert_id)
);

CREATE TABLE servicegroup_mpackname (

@adoroszlai adoroszlai Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table name is different in this schema (servicegroup_mpackname vs servicegroup_mpacknames).

@scottduan scottduan Feb 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, will change it.

service_group_id BIGINT NOT NULL,
service_group_cluster_id BIGINT NOT NULL,
mpack_name VARCHAR(255) NOT NULL,
CONSTRAINT PK_servicegroup_mpackname PRIMARY KEY (service_group_id, service_group_cluster_id, mpack_name),
FOREIGN KEY (service_group_id) REFERENCES servicegroups(service_group_id),
FOREIGN KEY (service_group_cluster_id) REFERENCES servicegroups(service_group_cluster_id)

@adoroszlai adoroszlai Feb 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constraints/foreign keys in this table are different than in the other DBMS schemas.

@scottduan scottduan Feb 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will update. Seems I forgot to update it.

);


CREATE INDEX idx_alert_history_def_id on alert_history(alert_definition_id);
CREATE INDEX idx_alert_history_service on alert_history(service_name);
CREATE INDEX idx_alert_history_host on alert_history(host_name);
Expand Down
9 changes: 9 additions & 0 deletions ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,15 @@ CREATE TABLE alert_notice (
FOREIGN KEY (history_id) REFERENCES alert_history(alert_id)
);

CREATE TABLE servicegroup_mpacknames (
service_group_id BIGINT NOT NULL,
service_group_cluster_id BIGINT NOT NULL,
mpack_name VARCHAR(255) NOT NULL UNIQUE,
CONSTRAINT PK_servicegroup_mpacknames PRIMARY KEY (service_group_id, service_group_cluster_id, mpack_name),
CONSTRAINT UQ_servicegroup_mpacknames UNIQUE (service_group_id, service_group_cluster_id),
CONSTRAINT FK_servicgroups FOREIGN KEY (service_group_id, service_group_cluster_id) REFERENCES servicegroups(id, cluster_id) ON DELETE CASCADE
);

CREATE INDEX idx_alert_history_def_id on alert_history(alert_definition_id);
CREATE INDEX idx_alert_history_service on alert_history(service_name);
CREATE INDEX idx_alert_history_host on alert_history(host_name);
Expand Down
9 changes: 9 additions & 0 deletions ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,15 @@ CREATE TABLE alert_notice (
FOREIGN KEY (history_id) REFERENCES alert_history(alert_id)
);

CREATE TABLE servicegroup_mpacknames (
service_group_id NUMBER(19) NOT NULL,
service_group_cluster_id NUMBER(19) NOT NULL,
mpack_name VARCHAR2(255) NOT NULL UNIQUE,
CONSTRAINT PK_servicegroup_mpacknames PRIMARY KEY (service_group_id, service_group_cluster_id, mpack_name),
CONSTRAINT UQ_servicegroup_mpacknames UNIQUE (service_group_id, service_group_cluster_id),
CONSTRAINT FK_servicgroups FOREIGN KEY (service_group_id, service_group_cluster_id) REFERENCES servicegroups(id, cluster_id) ON DELETE CASCADE
);

CREATE INDEX idx_alert_history_def_id on alert_history(alert_definition_id);
CREATE INDEX idx_alert_history_service on alert_history(service_name);
CREATE INDEX idx_alert_history_host on alert_history(host_name);
Expand Down
Loading