mesos / storm

Storm on Mesos!
Apache License 2.0
138 stars 66 forks source link

Storm v1.0.4 breaks storm-mesos #222

Closed erikdw closed 6 years ago

erikdw commented 6 years ago

Seems the TopologyDetails() constructor has changed from Storm v1.0.3 to Storm v1.0.4. Yet another non-obvious dependency between this project and Storm proper.

Built using this command:

% STORM_RELEASE=1.0.4 MESOS_RELEASE=0.28.2 bin/build-release.sh

Leading to these compilation errors:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/TestUtils.java:[47,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[64,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[89,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[98,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[105,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[121,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[129,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[138,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[213,28] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[247,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[255,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[265,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[294,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[301,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
[ERROR] /Users/eweathers/dev/third-party/storm-mesos/storm/src/test/storm/mesos/MesosCommonTest.java:[310,12] no suitable constructor found for TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.util.Map<org.apache.storm.scheduler.ExecutorDetails,java.lang.String>,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
    constructor org.apache.storm.scheduler.TopologyDetails.TopologyDetails(java.lang.String,java.util.Map,org.apache.storm.generated.StormTopology,int,java.lang.String) is not applicable
      (actual and formal argument lists differ in length)
JessicaLHartog commented 6 years ago

This was inadvertently broken by @revans2 in Storm in this commit: https://github.com/apache/storm/commit/44b80fbfcf8a52b6e7cb3840a52ff07f8a2364e4

Namely, owner is now a required parameter in all TopologyDetails constructors.

Notably we use TopologyDetails in a few places as seen here.

revans2 commented 6 years ago

@erikdw and @JessicaLHartog really sorry about that. I don't always know what is and isn't being used outside by other projects. Please file JIRA against storm and assign it to me. I will be happy to revert the change and make it so owner is optional in storm 1.x

erikdw commented 6 years ago

@revans2 : thanks for the message! I'm not sure what @JessicaLHartog's view is, but we should be able to workaround this particular clash pretty easily.

There is a much worse incompatibility we've encountered, as mentioned in #214 and STORM-2126:

Storm 1.1+ now requires Supervisors to exist before Slots can be passed around through the core Nimbus scheduling logic. This breaks a fundamental assumption of the storm-on-mesos implementation that Nathan Marz concocted -- that the storm-on-mesos plugin can create Slots in allSlotsAvailableForScheduling() without Supervisors existing yet. This is required because storm-on-mesos creates per-topology Supervisors on-the-fly as topologies get assigned onto Mesos worker hosts. The change in STORM-2126 was totally sensible, so I'm not saying that it was a bad change, just noting that it totally breaks storm-on-mesos.

It would be awesome if we could schedule a meeting with you and maybe some other deeply knowledgeable Storm contributors (e.g., from Yahoo and/or Hortonworks) to figure out how to better integrate Storm with Mesos. I think we'd all agree that having it done via this separate plugin is proving too error prone, but the question is more around how to technically run Storm on Mesos without the assumption above. Maybe sometime in January? Perhaps it would be better to just do a meeting between you and us to introduce the issue and do a bit of brainstorming on possible alternative methods for this integration.

revans2 commented 6 years ago

@erikdw please file the JIRA for the issues around STORM-2126 and assign it to me. If you could include some kind of a unit or integration test that would be great so we can avoid breaking you again in the future.

erikdw commented 6 years ago

@revans2 : agreed that it would be best to have a test of some sort! But at this juncture this project cannot function with the way Storm-1.1+ works, so until we can figure out how to workaround this fundamental incompatibility it wouldn't be possible to have such a test. Figuring out how to overcome this new-ish incompatibility (Storm requiring Supervisor before Slots, storm-on-mesos requiring Slots before Supervisors) sounds hard to me and is why I'm proposing a brainstorming meeting.

Notably for running on Mesos, Supervisors don't provide much value, ideally we wouldn't need them at all, but that is a really big change to Storm.

revans2 commented 6 years ago

@erikdw I agree that supervisors don't provide a lot of value in storm either. In 2.x though we can change a lot of the concepts in storm, and perhaps we can go a different route that is going to be better longer term.

The concept of a supervisor only exists in nimbus so the scheduler knows where it can/should put executors. RAS really needs some of this information to work properly, both in terms of locality and in terms of the resources available for the scheduler. When everything was just a slot and scheduling was round robin it was not a big deal, but that is no longer the case.

I have thought about this a bit and I think what we really want is a resource request API exposed to the storm scheduler. We can also provide some generic fall back code to make it work for existing schedulers.

The current assumption is that all resources are dedicated to storm and available right now, but that is not true. So if we can have some data structure that the storm scheduler can return that would give options for what it needs/wants to be able to run everything, and similarly something that can set some expectations for the scheduler about the size and type of resources it might be able to get access to.

So if a cluster runs out of resources and the storm scheduler cannot schedule all of topology B, it can tell a higher level scheduler that topology B needs 12 more slots with 4096 MB of memory and 400% CPU each, preferably try to put them close to host X (because part of the topology is already there). The priority for this ask is higher then the ask for topology C.

The generic fallback code can then look at all of the topologies that are not scheduled and have no ask, then it can try to generate a fairly generic ask for them.

If we really want to go crazy we could look restricting existing "supervisors" to optionally only specific users or topologies, so in the case of YARN we could have nimbus act as a proxy user and launch an unmanaged AM for each topology.

The initial changes for the ask API should not be too hard, and I think this is something that I can use for elasticity as well. What do you think?

erikdw commented 6 years ago

@revans2 : you've given this a lot of thought already! It sounds like this is going the right direction for Storm to integrate much better with Mesos than it does today. We'd love to work with you on ensuring the underlying APIs are proper for Mesos as much as our work schedules will allow (we'll have to fight for time within our company as there are competing high priority initiatives).

BTW, one wrinkle to keep in mind is that Mesos is inverted from YARN in terms of resource "requesting": in standard Mesos usage applications do not request specific amounts of resources from the Mesos cluster. Instead applications are offered available resources from the Mesos cluster and then the application chooses to accept the offers or decline them. There is a concept of dynamic reservations which might allow for a more intuitive fit for this "ask API", but we haven't messed with that yet.

P.S., I think that Storm not working on Mesos for Storm v1.1->v1.N is acceptable, as long as we can figure out a plan for getting it to work on some future version of Storm (e.g., v2+), which is what your proposal would allow for.

revans2 commented 6 years ago

@erikdw yes it is all about finding time vs our current priorities right now. But we have been really looking at the scheduler lately so I am hopeful to be able to "sneak" it in as a part of our work on elasticity.