Skip to content
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.

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

Fix MesosSupervisor.getMetadata #121

Merged
merged 1 commit into from
Apr 5, 2016
Merged

Conversation

erikdw
Copy link
Collaborator

@erikdw erikdw commented Apr 2, 2016

This is intended to fix issue #119.

With the introduction of the TaskAssignments refactor for creating unique
task IDs (#106), I introduced a couple of bugs in the implementation of
MesosSupervisor.getMetadata:

  1. The slot counts in the Storm UI were broken -- the return from
    getMetadata was always a single element vector, due to using a
    Set instead of a java array previously. This was causing the
    PersistentVector.create(Object ... object) method to be matched,
    which just puts the passed objects into a vector without iterating
    over their constituent elements. Since we are passing a single
    Set object, we are getting a single element in the resultant vector.
    So the fix is to just create a List and pass that to
    PersistentVector.create().
  2. The returned Object must be serializable. Depending on the
    build and runtime environment, the serialization done by the storm
    supervisor during initialization will fail, crashing the supervisor.
    That was happening because we were passing back the
    ConcurrentHashMap$KeySetView object, which is not serializable.
    Here too, the fix is to just create a List and pass that to
    PersistentVector.create().

NOTE: I haven't been able to reproduce problem 2. unfortunately.

@erikdw erikdw force-pushed the fix-getMetadata branch 2 times, most recently from 67310e4 to 3358b67 Compare April 2, 2016 07:37
This is intended to fix issue mesos#119.

With the introduction of the TaskAssignments refactor for creating unique
task IDs (mesos#106), I introduced a couple of bugs in the implementation of
MesosSupervisor.getMetadata:

1. The slot counts in the Storm UI were broken -- the return from
   getMetadata was always a single element vector, due to using a
   Set instead of a java array previously. This was causing the
   PersistentVector.create(Object ... object) method to be matched,
   which just puts the passed objects into a vector without iterating
   over their constituent elements.  Since we are passing a single
   Set object, we are getting a single element in the resultant vector.
   So the fix is to just create a List and pass that to
   PersistentVector.create().
2. The returned Object must be serializable.  Depending on the
   build and runtime environment, the serialization done by the storm
   supervisor during initialization will fail, crashing the supervisor.
   That was happening because we were passing back the
   ConcurrentHashMap$KeySetView object, which is not serializable.
   Here too, the fix is to just create a List and pass that to
   PersistentVector.create().

NOTE: I haven't been able to reproduce problem 2. unfortunately.
@drewrobb
Copy link
Contributor

drewrobb commented Apr 5, 2016

@erikdw this fixes problem 2 for me, thank you!

@erikdw erikdw merged commit a07f7e2 into mesos:master Apr 5, 2016
@erikdw erikdw deleted the fix-getMetadata branch April 5, 2016 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants