You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2014/11/14 23:50:52 UTC

[GitHub] incubator-brooklyn pull request: Start entity fix + Catalog item c...

GitHub user neykov opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/331

    Start entity fix + Catalog item comparator fix

    A couple of fixes after testing.
    
    Depends on #314 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/neykov/incubator-brooklyn fix/testing-fixes

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/331.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 #331
    
----
commit 442a769bac089e199b120f296f9b186040667243
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-06T11:06:10Z

    make heartbeat timeout (to trigger failover) and poll period configurable from brooklyn.properties

commit 0773c64fabe89e8af0a48d570a461d2e30ce133e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-07T08:38:24Z

    clean up copyPersistedState code
    
    move shared code to new `BrooklynPersistenceUtils`,
    make a new `LocationWithPersistenceStore.newPersistenceObjectStore` interface,
    and add documentation for `copy-state` command

commit 3e793cd7b67e95db09a68a4e11a16e84e6a117cb
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-10T14:18:47Z

    support for exporting persistent state

commit 38a756b2bb08f2baa80dddb53faa51cc645f988b
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-10T21:26:42Z

    more persistence, mainly tidying code to treat all BrooklynObject instances the same

commit 7532be8c87e2791a6e3b056067ce309cd146641d
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-11T10:41:03Z

    Create backup of persistent stores at key times
    
    Created on master promotion (technically just before persistence starts) by reading from the store,
    and on demotion by HA subsystem by reading from local in-memory model.
    
    Includes new BrooklynServerPaths cleaning up logic from BrooklynServerConfig, and some cleanups to the utils in BrooklynPersistenceUtils.
    Relevant config keys are the *PERSISTENCE*BACKUP* options in BrooklynServerConfig.
    
    Logging now looks like this:
    
         2014-11-11 13:46:06,153 INFO  Rebinding from /Users/alex/.brooklyn/brooklyn-persisted-state/data...
         2014-11-11 13:46:06,409 INFO  Rebind VanillaSoftwareProcessImpl{id=wGwL3QXG} connecting to pre-running service
         2014-11-11 13:46:06,606 INFO  Rebind complete (MASTER) in 669ms: 1 app, 2 entities, 14 locations, 0 policies, 9 enrichers, 0 feeds, 0 catalog items
        *2014-11-11 13:46:06,697 INFO  Back-up of persisted state created on promotion, in /Users/alex/.brooklyn/brooklyn-persisted-state/data/backups/2014-11-11-1346-oZyOou3V-promotion-s5kE
         2014-11-11 13:46:26,362 INFO  REST using security provider brooklyn.rest.security.provider.BrooklynUserWithRandomPasswordSecurityProvider
         2014-11-11 13:46:26,363 INFO  Allowing access to web console from localhost or with brooklyn:Ao3m5nZoe9
        *2014-11-11 13:46:46,825 INFO  Back-up of persisted state created on demotion, in /Users/alex/.brooklyn/brooklyn-persisted-state/data/backups/2014-11-11-1346-oZyOou3V-demotion-XYlN
         2014-11-11 13:46:47,045 WARN  Management node oZyOou3V detected master change, from  (?) to orZg7gai (1415713606000 / 1.05s ago) http://almacretin.local:8082/
         2014-11-11 13:46:47,059 INFO  Rebind complete (HOT_STANDBY, iteration 0) in 220ms: 1 app, 2 entities, 14 locations, 0 policies, 9 enrichers, 0 feeds, 0 catalog items
    
    In future it would be nice to have periodic backups and some automatic clean-out support ... but for another day!

commit f39c6a3fc3f40cd3469c38b7d61908d317c1d7de
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-12T09:02:42Z

    Use a linear range QuorumCheck to fail if there are too many dangling references.
    
    This introduces QuorumChecks.newLinearRange which allows specifying points in a place
    to define a set of line segments defining the quorum.
    This can be TypeCoerced from a string, e.g. `[[5,5],[10,10],[100,70],[200,140]]`
    (with things like `all` and `atLeastOne` also supported).
    
    Such a range is then used to cause rebind to fail if there are a lot of dangling references.
    The precise number is allowing 2 for small values, scaling to 5% at big values,
    but this can be tweaked using RebindManagerImpl.DANGLING_REFERENCES_MIN_REQUIRED_HEALTHY
    aka rebind.failureMode.danglingRefs.minRequiredHealthy .  (e.g. set this to all in
    brooklyn.properties to cause rebind to fail if there are any dangling references;
    but note this can occasionally fail if something is deleted as different items are
    not persisted simultaneously).
    
    Also:
    * There are also some tidies to how `RebindContext` is used, since many of the
      dangling references weren't being caught
    * We are stricter about persisting proxies, since that doesn't work (type written as `$Proxy16`)
    * We tolerate remote timestamps in persisted objects (since these can occur in backups, currently);
      we don't *use* those timestamps for anything (except in tests when they can be `prefer`red),
      but there is no harm in having them, if someone copied across backup persistence files

commit 9dd1a95724a473b59a777fb6d627b6dde2fa0547
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T02:05:16Z

    Add a HOT_BACKUP mode where nodes are read-only copies but NOT willing to be master.

commit b9c1b6fca99165f62b712ec7fd7967f7e4b4e283
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T03:42:54Z

    Make metrics on rebind, persistence, and HA available through REST API.
    
    So that a management plane or human can determine server health and historic problems.

commit f90faf4b1a9838b726d1ccec76fc2acb2304df47
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T04:50:23Z

    a few more goodies, coercing strings and maps to location and other fixes
    
    where new Object() was used in persistence, it would not serialize nicely, and also it would not deserialize to the same (NONE instance) value; fix the logic for BasicParameterType defaults including null, and then when testing discovered problems if we passed Strings in, so fix that

commit f6cd031428f2f355d56e6eedef14fcabb0abd8e5
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T15:33:31Z

    code review to #314, use enum and create unmanaged location

commit ed805673797a032d002aa941a3e2eab32aef9881
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T17:34:24Z

    specify a filename when people download service state

commit 9edd58db490863e3e34758b31a19e3cb1ed8deae
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T17:44:00Z

    fail fast if invalid mode given via REST API

commit a6891c691a3ead9ef0dac4e978527e034a83e576
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T21:42:04Z

    correct description and tidied code for determining backup location spec v non-backup location spec, fixing NPE's

commit 5060c4f06e80a1b7b8062852a276a8d32bd12237
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T21:42:36Z

    fix use of jclouds blob store wrapper when "container" contains a /, with test, fixing errors with making backups in SL

commit 24a3c3417f0ad2c60d3acc2328b6a437d2dfdacf
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-11-13T23:35:15Z

    final fixes from code review of #314, hot_backup
    
    fixes state transitions (master -> hot_backup etc) and some better logging around catalog

commit 1a4e8539dbc89e8d7f2e818c79611f72bc35c334
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-11-14T14:05:18Z

    Fix generated zip entries - use supported argument.
    
    Seem the ArchiveBuilder can't create archives without root-level folder, pass an actual folder name.

commit fa7ba6f838a488ed782d2cacf99a04687a55821f
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-11-14T14:06:45Z

    Use supported names for object store
    
    Fixes:
      * Files are now accessible (not 404) from Cyberduck & Softlayer UI
      * There's no longer a phantom folder with the same name as the parent for backups.

commit 253583e8cc68c8edf8bee5c725497bd393b8be02
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-11-14T22:13:22Z

    Catalog item comparator - implement custom
    
    The NaturalOrderComparator leads to unexpected sort order (i.e 0.0.0_SNAPSHOT and 0.0.1-SNAPSHOT-20141111114709760). OSGi's Version class and other semver java implementations are too strict.

commit 5868b7d65a247512cb14d78f0406ca7de52eb508
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date:   2014-11-14T22:14:28Z

    Fix NPE on creating entities with no explicit location

----


---
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] incubator-brooklyn pull request: Start entity fix + Catalog item c...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/331


---
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] incubator-brooklyn pull request: Start entity fix + Catalog item c...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/331#issuecomment-63179436
  
    > if i understand correctly, splits on and otherwise ignores non-digits
    
    It doesn't ignore them. If both parts are non-digits then compares plain strings (L80). 
    
    >(and use the natural order comparator so rc10 > rc1 !)
    
    Yes, definitely should use the natural order comparator for strings instead.
    
    > it would also be sensible to have special logic that, given two versions identical up to a qualifier we prefer the non-qualified item
    
    I though the logic should be the reverse (as it is now) - that's what I remember users expect? For example 3.3.0-200141115 beats 3.3.0.
    
    > NaturalOrderComparator sorts 0.0.0_SNAPSHOT higher than 0.0.1-SNAPSHOT-20141111114709760
    
    Yes, didn't expect it but turns out that's the case. That's actually the reason why I decided to go with a separate comparator. First tried existing version parsers - OSGi's provided Version class and also an external dependency - semver which provides semantic Version class. Both failed to parse some of the version strings we have (underscore is not supported as a delimiter in semver, osgi requires a dot delimiter). They are just too strict for our use.



---
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] incubator-brooklyn pull request: Start entity fix + Catalog item c...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/331#issuecomment-63152574
  
    last two commits look pretty good - merging.
    
    not convinced we have the catalog sorting strategy right, but we can refine it.
    
    * this approach, if i understand correctly, splits on and otherwise ignores non-digits.  but that could be surprising.  if i have `beta1` and `rc2` i'd be pleased that `rc2` wins, even if it's for alphabetic reasons; but if i then release `beta3` i'd be surprised to see it come first.  we should include alphabetic items in the items we sort on (and use the natural order comparator so rc10 > rc1 !)
    
    * it would also be sensible to have special logic that, given two versions identical up to a qualifier we prefer the non-qualified item -- ie prefer `0.3.0` over `0.3.0_rc1`.  (but we should be careful *not* to prefer `1` over `1.3` !)
    
    * also your commit message suggests that `NaturalOrderComparator` sorts `0.0.0_SNAPSHOT` higher than `0.0.1-SNAPSHOT-20141111114709760` -- i'm surprised by that, is there a good reason?  however i agree with the spirit that `_` and `-` should not be significant for determining sort order here and i can see other areas where it might cause problems (minor ones, but still...)


---
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] incubator-brooklyn pull request: Start entity fix + Catalog item c...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/331#issuecomment-63180061
  
    very surprised about NOC underscore treatment.  sounds like a bug.
    
    one common strategy is that the qualifier is used to indicate pre-release (`3.3.0` is better than `3.3.0-rc1`); if not, i'd say the qualifier should *always* be part of the release (`3.3.0_201311` v `3.3.0_201404`).  with `3.3.0` and `3.3.0-200141115` i wouldn't have a clue which is more recent (so fallback to the first interpretation is good).


---
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.
---