You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anton-mironenko <gi...@git.apache.org> on 2017/08/02 14:40:44 UTC
[GitHub] geode pull request #677: GEODE-3038: A server process shuts down quietly whe...
GitHub user anton-mironenko opened a pull request:
https://github.com/apache/geode/pull/677
GEODE-3038: A server process shuts down quietly when path to cache.xml is incorrect
[GEODE-3038](https://issues.apache.org/jira/browse/GEODE-3038)
The error
"Declarative Cache XML file/resource \[path-to-cache-xml\] does not exist"
is trying to be written into the log in GemFireCacheImpl.basicCreate(), after method GemFireCacheImpl.close() is called, so that writing into the log is already not available.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [x] Is your initial contribution a single, squashed commit?
- [x] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anton-mironenko/geode develop
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/677.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #677
----
commit f119fb381d4212e8714d26f2388860206daa3d8a
Author: Anton Mironenko <an...@gmail.com>
Date: 2017-08-01T12:18:12Z
write cachexml-not-found exception into the log in a proper place
commit bf0b059035751b3d7cba82a145546edb7efce824
Author: Anton Mironenko <an...@gmail.com>
Date: 2017-08-01T12:52:14Z
Merge remote-tracking branch 'upstream/develop' into GEODE-3038
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #677: GEODE-3038: A server process shuts down quietly whe...
Posted by anton-mironenko <gi...@git.apache.org>.
Github user anton-mironenko commented on a diff in the pull request:
https://github.com/apache/geode/pull/677#discussion_r131154689
--- Diff: geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ---
@@ -1208,6 +1208,9 @@ private void initialize() {
this.system.getConfig());
initializeDeclarativeCache();
completedCacheXml = true;
+ } catch (CacheXmlException e) {
--- End diff --
@dschneider-pivotal Thank you for your feedback. I've replaced CacheXmlException with RuntimeException. Sorry for two duplicate commits instead of the only one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #677: GEODE-3038: A server process shuts down quietly whe...
Posted by dschneider-pivotal <gi...@git.apache.org>.
Github user dschneider-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/677#discussion_r131003885
--- Diff: geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ---
@@ -1208,6 +1208,9 @@ private void initialize() {
this.system.getConfig());
initializeDeclarativeCache();
completedCacheXml = true;
+ } catch (CacheXmlException e) {
--- End diff --
It looks like a number of exceptions can be thrown from initializeDeclarativeCache.
Would it be better to catch, log, and rethrow RuntimeException instead of CacheXmlException?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
RE: [GitHub] geode issue #677: GEODE-3038: A server process shuts
down quietly when path ...
Posted by Anton Mironenko <am...@amdocs.com>.
Hello,
How can we go further with this pull request?
Somehow my latest comment was not published in the mailing list:
" I've added the test org.apache.geode.cache30.CacheXmlNotFoundUnitTest#testCacheXmlNotFoundInRealLog.
It tests that an error about missing cache-xml file is indeed printed in the text log file, specified via "log-file" parameter.
The existing test org.apache.geode.cache30.CacheXml66DUnitTest#testNonExistentFile() is supposed to test the same,
but actually it doesn't. It only checks for an CacheXmlException exception to be thrown."
BR,
Anton
-----Original Message-----
From: anton-mironenko [mailto:git@git.apache.org]
Sent: Tuesday, August 29, 2017 20:10
To: dev@geode.apache.org
Subject: [GitHub] geode issue #677: GEODE-3038: A server process shuts down quietly when path ...
Github user anton-mironenko commented on the issue:
https://github.com/apache/geode/pull/677
Well, this test category is DistributedTest, not Test. This is why I didn't see it in regular unit tests run.
Now the question is - why this test doesn't fail, whereas it should fail without my fix.
---
If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastructure@apache.org or file a JIRA ticket with INFRA.
---
This message and the information contained herein is proprietary and confidential and subject to the Amdocs policy statement,
you may review at https://www.amdocs.com/about/email-disclaimer <https://www.amdocs.com/about/email-disclaimer>
[GitHub] geode issue #677: GEODE-3038: A server process shuts down quietly when path ...
Posted by anton-mironenko <gi...@git.apache.org>.
Github user anton-mironenko commented on the issue:
https://github.com/apache/geode/pull/677
Well, this test category is DistributedTest, not Test. This is why I didn't see it in regular unit tests run.
Now the question is - why this test doesn't fail, whereas it should fail without my fix.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #677: GEODE-3038: A server process shuts down quietly when path ...
Posted by anton-mironenko <gi...@git.apache.org>.
Github user anton-mironenko commented on the issue:
https://github.com/apache/geode/pull/677
Actually there is already a test which tests exactly this bug:
org.apache.geode.cache30.CacheXml66DUnitTest#testNonExistentFile
But I see that all the package org.apache.geode.cache30 is not included now into unit tests run. What is the best way to start this test being executed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #677: GEODE-3038: A server process shuts down quietly when path ...
Posted by anton-mironenko <gi...@git.apache.org>.
Github user anton-mironenko commented on the issue:
https://github.com/apache/geode/pull/677
Hello, is there anything I can do to proceed with this pull request?
I was asked to put RuntimeException instead of CacheXmlException. This is what I did.
This PR contains not-squashed commits, also this PR is done in the main stream "develop" in my forked repository on GitHub, instead of being done in the proper branch "GEODE-3038".
May be these are the reasons, why my PR stuck? So I could create another PR with the only one squashed commit from the right branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #677: GEODE-3038: A server process shuts down quietly when path ...
Posted by anton-mironenko <gi...@git.apache.org>.
Github user anton-mironenko commented on the issue:
https://github.com/apache/geode/pull/677
@dschneider-pivotal Thank you for your feedback. I've replaced CacheXmlException with RuntimeException. Sorry for two duplicate commits instead of the only one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---