Skip to content

[AMBARI-23129] Multiple issues while executing Ambari server upgrade to Ambari 2.7.0#608

Merged
rlevas merged 2 commits intoapache:trunkfrom
rlevas:AMBARI-23129
Mar 10, 2018
Merged

[AMBARI-23129] Multiple issues while executing Ambari server upgrade to Ambari 2.7.0#608
rlevas merged 2 commits intoapache:trunkfrom
rlevas:AMBARI-23129

Conversation

@rlevas
Copy link
Contributor

@rlevas rlevas commented Mar 9, 2018

What changes were proposed in this pull request?

1. An NPE is thrown will initializing the Ambari server upgrade catalog

com.google.inject.persist.jpa.JpaPersistService#begin

    public void begin() {
        Preconditions.checkState(null == this.entityManager.get(), "Work already begun on this thread. Looks like you have called UnitOfWork.begin() twice without a balancing call to end() in between.");
        this.entityManager.set(this.emFactory.createEntityManager());
    }

this.emFactory is null.

Cause
com.google.inject.persist.jpa.JpaPersistService#start() was not called before com.google.inject.persist.jpa.JpaPersistService#begin to do order of operations in org.apache.ambari.server.upgrade.SchemaUpgradeHelper#main.

Solution
Ensure com.google.inject.persist.jpa.JpaPersistService#start() is being called before com.google.inject.persist.jpa.JpaPersistService#begin in org.apache.ambari.server.upgrade.SchemaUpgradeHelper#main.


2. Missing repo_os, repo_definition, and repo_tags tables

The repo_os, repo_definition, and repo_tags tables were never added to the UpgradeCatalog implementation. The migration logic is also needed.

Solution
Add the missing tables and logic while executing org.apache.ambari.server.upgrade.UpgradeCatalog270#executeDDLUpdates.


3. Entity classes are initialized before the schema of the underlying database is updated

Solution
Notify relevant classes that the persistence infrastructure is ready after DDL updates have been applied.

The org.apache.ambari.server.events.publishers.AmbariEventPublisher is to be used for issuing a org.apache.ambari.server.events.JpaInitializedEvent.


4. JVM does not exit after performing upgrade

After executing ambari-server upgrade, the JVM process hangs and does not exit. According to the logs, no errors have occurred.

Cause
The cause of this is several non-daemon threads not being shutdown by Ambari code.

Solution
Change relevant non-daemon threads to daemon threads and ensure any thread polls are not forcing one thread to be alive at all times.

How was this patch tested?

Ran unit tests.
Executed Ambari upgrade
Executed Ambari, started and stop services

Please review Ambari Contributing Guide before opening a pull request.

@Inject
@Transactional
void loadClustersAndHosts() {
private void loadClustersAndHosts() {
Copy link
Contributor

@mpapirkovskyy mpapirkovskyy Mar 9, 2018

Choose a reason for hiding this comment

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

Guice AOP interceptors don't work on private methods.
This means that Transactional annotation will be ignored.
Please check what is correct behavior here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is a transaction even relevant here? The method does a bunch of DAO reads. @jonathan-hurley?

Copy link
Contributor

Choose a reason for hiding this comment

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

Theres RequiresSession annotation to mimic read-only transaction behavior.
Basically if session is not already opened on method call, each DAO read will create own session.
Not really relevant here since this method is not called frequently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. true. I can add the RequiresSession annotation if we think it is needed. I removed @Transactional

Copy link
Member

Choose a reason for hiding this comment

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

That @Transactional was added a long, long time ago by Myroslav - I agree it's not needed.

@asfgit
Copy link

asfgit commented Mar 10, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/1076/
Test FAILed.
Test FAILured.

@asfgit
Copy link

asfgit commented Mar 10, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/1077/
Test FAILed.
Test FAILured.

@adoroszlai
Copy link
Contributor

retest this please

@asfgit
Copy link

asfgit commented Mar 10, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/1092/
Test PASSed.

@rlevas rlevas merged commit 249af2b into apache:trunk Mar 10, 2018
@rlevas rlevas deleted the AMBARI-23129 branch March 19, 2018 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants