Skip to content

[AMBARI-22992] Update error handling during mpack installation #363

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
mradha25 merged 3 commits into from
Feb 15, 2018
Merged

[AMBARI-22992] Update error handling during mpack installation #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a8786f
[AMBARI-22992] Update error handling during mpack installation
mradha25 Feb 14, 2018
e8b320d
[AMBARI-22997] cluster-settings.xml is not present in /var/lib/ambari…
mradha25 Feb 14, 2018
956ae7b
[AMBARI-22992] Update error handling during mpack installation - revi…
mradha25 Feb 15, 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
5 changes: 5 additions & 0 deletions ambari-server/src/main/assemblies/server.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,11 @@
<source>conf/unix/ca.config</source>
<outputDirectory>/var/lib/ambari-server/keys</outputDirectory>
</file>
<file>
<fileMode>755</fileMode>
<source>target/classes/cluster-settings.xml</source>
<outputDirectory>/var/lib/ambari-server/resources</outputDirectory>
</file>
<file>
<fileMode>755</fileMode>
<source>target/classes/Ambari-DDL-Postgres-CREATE.sql</source>
Expand Down
53 changes: 16 additions & 37 deletions ambari-server/src/main/java/org/apache/ambari/server/mpack/MpackManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.apache.ambari.server.state.Module;
import org.apache.ambari.server.state.Mpack;
import org.apache.ambari.server.state.OsSpecific;
import org.apache.ambari.server.state.StackId;
import org.apache.ambari.server.state.stack.StackMetainfoXml;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
Expand Down Expand Up @@ -208,13 +207,13 @@ public MpackResponse registerMpack(MpackRequest mpackRequest)
if (mpackId != null) {
mpackMap.put(mpackId, mpack);
mpack.setMpackId(mpackId);
populateStackDB(mpack);
return new MpackResponse(mpack);
} else {
String message = "Mpack :" + mpackRequest.getMpackName() + " version: " + mpackRequest.getMpackVersion()
+ " already exists in server";
throw new ResourceAlreadyExistsException(message);
if (populateStackDB(mpack))
return new MpackResponse(mpack);
}
String message = "Mpack :" + mpackRequest.getMpackName() + " version: " + mpackRequest.getMpackVersion()
+ " already exists in server";
throw new ResourceAlreadyExistsException(message);

}

/***
Expand Down Expand Up @@ -433,17 +432,8 @@ private Boolean createMpackDirectory(Mpack mpack)
*/
private void createSymLinks(Mpack mpack) throws IOException {

String stackId = mpack.getStackId();
String stackName = "";
String stackVersion = "";
if (stackId == null) {
stackName = mpack.getName();
stackVersion = mpack.getVersion();
} else {
StackId id = new StackId(stackId);
stackName = id.getStackName();
stackVersion = id.getStackVersion();
}
String stackName = mpack.getName();
String stackVersion = mpack.getVersion();
File stack = new File(stackRoot + "/" + stackName);
Path stackPath = Paths.get(stackRoot + "/" + stackName + "/" + stackVersion);
Path mpackPath = Paths.get(mpacksStaging + "/" + mpack.getName() + "/" + mpack.getVersion());
Expand Down Expand Up @@ -517,8 +507,8 @@ protected Long populateDB(Mpack mpack) throws IOException {
String mpackName = mpack.getName();
String mpackVersion = mpack.getVersion();
List resultSet = mpackDAO.findByNameVersion(mpackName, mpackVersion);

if (resultSet.size() == 0) {
StackEntity stackEntity = stackDAO.find(mpackName, mpackVersion);
if (resultSet.size() == 0 && stackEntity == null) {
LOG.info("Adding mpack {}-{} to the database", mpackName, mpackVersion);

MpackEntity mpackEntity = new MpackEntity();
Expand All @@ -540,20 +530,10 @@ protected Long populateDB(Mpack mpack) throws IOException {
* @param mpack
* @throws IOException
*/
protected void populateStackDB(Mpack mpack) throws IOException {

String stackId = mpack.getStackId();
String stackName = "";
String stackVersion = "";
if (stackId == null) {
stackName = mpack.getName();
stackVersion = mpack.getVersion();
} else {
StackId id = new StackId(stackId);
stackName = id.getStackName();
stackVersion = id.getStackVersion();
}
protected boolean populateStackDB(Mpack mpack) throws IOException {

String stackName = mpack.getName();
String stackVersion = mpack.getVersion();
StackEntity stackEntity = stackDAO.find(stackName, stackVersion);
if (stackEntity == null) {
LOG.info("Adding stack {}-{} to the database", stackName, stackVersion);
Expand All @@ -563,11 +543,10 @@ protected void populateStackDB(Mpack mpack) throws IOException {
stackEntity.setStackVersion(stackVersion);
stackEntity.setMpackId(mpack.getMpackId());
stackDAO.create(stackEntity);
return true;
} else {
LOG.info("Updating stack {}-{} to the database", stackName, stackVersion);

stackEntity.setMpackId(mpack.getMpackId());
stackDAO.merge(stackEntity);
LOG.error("Stack {}-{} already exists in the database", stackName, stackVersion);
Copy link
Contributor

@jayush jayush Feb 15, 2018

Choose a reason for hiding this comment

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

We should through an exception here. Infact we should add a precheck to ensure that corresponding stack doesn't exist in the DB BEFORE creating an mpack DB entry.

Copy link
Contributor Author

@mradha25 mradha25 Feb 15, 2018

Choose a reason for hiding this comment

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

Hi Jayush, since the flow during registration is always populateMpackDB and then populateStackDB, there is a check in populateMpackDB to see if the entry already exists in table mpacks, only then it is added. While deleting an mpack also first stackEntry is removed and then the MpackEntry. Will that not take care of the scenario? Kindly advise. I will modify code to throw an exception instead of Logging an error.

return false;
}
}

Expand Down