Skip to content

STORM-1345: UpdateTopology API and implementation. - #922

Closed
Parth-Brahmbhatt wants to merge 1 commit into
apache:masterfrom
Parth-Brahmbhatt:STORM-1345
Closed

STORM-1345: UpdateTopology API and implementation.#922
Parth-Brahmbhatt wants to merge 1 commit into
apache:masterfrom
Parth-Brahmbhatt:STORM-1345

Conversation

@Parth-Brahmbhatt

Copy link
Copy Markdown
Contributor

No unit tests added. Update will allow to update config, jar (if some dependency is updated without the topology changing in any way), or the topology change it self.

@unsleepy22

Copy link
Copy Markdown

I have one question, since all worker must be restarted, what's the point of 'update'?
I suggest there should be at least two kinds of 'update':

  1. update application's config dynamically without restarting the topology, which is what JStorm has done.
  2. update topology's config, which requires restarting.

@revans2

Copy link
Copy Markdown
Contributor

@Parth-Brahmbhatt how far along is this patch with testing/work? I see a lot of commented out code in the pull request and I am having a little bit of difficulty understanding why. There is also at least one big TODO in the code. Could you please either remove the unneeded code or explain what the next steps are to use it and to deal with the TODO?

Also could you give a brief overview of the design of this. From looking at the code, including the commented out sections, it appears that the intentions is to upload a new jar, config, and/or topology layout. Then rebalance the topology to have it rescheduled. The supervisors also now look at a new topology version field to see if they should kill/restart the worker or not. Is that correct?

The blob store feature moves the conf, topology, and topology-jar into the blob store which also supports doing dynamic updates within the supervisor. I think it would be a lot cleaner if we could try and leverage this functionality.

I also agree with @unsleepy22 that there are several types of updates, and going off of your TODO in the code I think there are several different ways we want to potentially update a topology. There are changes that require a restart of the worker and ones that do not (but may require changes to the system/user code to allow this). There are also changes that should be done as a rolling upgrade, one worker at a time, to allow for minimal impact, but there are others that are so big that you are killing off the topology and just restarting it again, but keeping the ID for some reason.

I am fine with this being a first step where we only support a single type of update, which is the most generic of them. You can change anything and we will restart the entire topology as quickly as possible. But I would like to understand you view about where this feature ultimately ends up.

@Parth-Brahmbhatt

Copy link
Copy Markdown
ContributorAuthor

@revans2@unsleepy22 I did not know Jstorm has it and hasn't had a chance to look at the dist cache.

My goal was to allow users to update topology or config or jar and support workers being restarted in a rolling fashion, When the topology it self is changed we can still do rolling restart and that is the part where I have a TODO that I need to finish.

I thought all of those required workers to bounce, I am not sure which parts can work without bouncing the worker.

The code I have commented is about making update a state just like rebalance/kill/activate and I am still not convinced that it needs to be a state of its own.

I am fine with waiting till we are done with JStorm integration as long as JStorm achieves the same goals. If others think that this is an important enough feature that we should include in the release before we do a feature lockdown, I can upmerge with dist cache and use it as @revans2 suggested.

d2r pushed a commit to d2r/storm that referenced this pull request Oct 16, 2018
We are closing stale Pull Requests to make the list more manageable.
Please re-open any Pull Request that has been closed in error.
Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469
d2r pushed a commit to d2r/storm that referenced this pull request Oct 16, 2018
We are closing stale Pull Requests to make the list more manageable.
Please re-open any Pull Request that has been closed in error.
Closesapache#608Closesapache#639Closesapache#640Closesapache#648Closesapache#662Closesapache#668Closesapache#692Closesapache#705Closesapache#724Closesapache#728Closesapache#730Closesapache#753Closesapache#803Closesapache#854Closesapache#922Closesapache#986Closesapache#992Closesapache#1019Closesapache#1040Closesapache#1041Closesapache#1043Closesapache#1046Closesapache#1051Closesapache#1078Closesapache#1146Closesapache#1164Closesapache#1165Closesapache#1178Closesapache#1213Closesapache#1225Closesapache#1258Closesapache#1259Closesapache#1268Closesapache#1272Closesapache#1277Closesapache#1278Closesapache#1288Closesapache#1296Closesapache#1328Closesapache#1342Closesapache#1353Closesapache#1370Closesapache#1376Closesapache#1391Closesapache#1395Closesapache#1399Closesapache#1406Closesapache#1410Closesapache#1422Closesapache#1427Closesapache#1443Closesapache#1462Closesapache#1468Closesapache#1483Closesapache#1506Closesapache#1509Closesapache#1515Closesapache#1520Closesapache#1521Closesapache#1525Closesapache#1527Closesapache#1544Closesapache#1550Closesapache#1566Closesapache#1569Closesapache#1570Closesapache#1575Closesapache#1580Closesapache#1584Closesapache#1591Closesapache#1600Closesapache#1611Closesapache#1613Closesapache#1639Closesapache#1703Closesapache#1711Closesapache#1719Closesapache#1737Closesapache#1760Closesapache#1767Closesapache#1768Closesapache#1785Closesapache#1799Closesapache#1822Closesapache#1824Closesapache#1844Closesapache#1874Closesapache#1918Closesapache#1928Closesapache#1937Closesapache#1942Closesapache#1951Closesapache#1957Closesapache#1963Closesapache#1964Closesapache#1965Closesapache#1967Closesapache#1968Closesapache#1971Closesapache#1985Closesapache#1986Closesapache#1998Closesapache#2031Closesapache#2032Closesapache#2071Closesapache#2076Closesapache#2108Closesapache#2119Closesapache#2128Closesapache#2142Closesapache#2174Closesapache#2206Closesapache#2297Closesapache#2322Closesapache#2332Closesapache#2341Closesapache#2377Closesapache#2414Closesapache#2469
Sign up for freeto 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.

3 participants

@Parth-Brahmbhatt@unsleepy22@revans2