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

Next Incompatible Release Planning (v1.10) #2827

Closed
13 tasks
ron-murhammer opened this issue Jan 11, 2018 · 41 comments
Closed
13 tasks

Next Incompatible Release Planning (v1.10) #2827

ron-murhammer opened this issue Jan 11, 2018 · 41 comments
Labels
Discussion team communication thread, meant for coordination and decision making

Comments

@ron-murhammer
Copy link
Member

ron-murhammer commented Jan 11, 2018

So once we cut the next stable (hopefully in the next few days), I'd like to turn attention to what items folks would like to see in the next incompatible release. I think it would be good to get a check list together so we can have a sense of what people want to focus on. My initial thought is target a 3 month period and see where that gets us.

@ron-murhammer ron-murhammer added Discussion team communication thread, meant for coordination and decision making requires major release labels Jan 11, 2018
@RoiEXLab
Copy link
Member

Should #2614 be closed in favour of this?

@ron-murhammer
Copy link
Member Author

@RoiEXLab Maybe best to summarize #2614 based on the conversation there and add to the initial post here? Given that the thread is 2 months old, probably better to get a fresh start :)

@RoiEXLab
Copy link
Member

@ron-murhammer Most of the discussion was if we fork off a "stable" branch we could submit hotfixes to, in case a P1 or P0 issue is encountered

@RoiEXLab
Copy link
Member

@ron-murhammer I updated your list with a minor entry

@RoiEXLab
Copy link
Member

@ron-murhammer Another task we should do after we decided to make incompatible changes:
If it's possible, we should consider bumping https://github.com/triplea-game/triplea/blob/master/latest_version.properties to the latest version String if that's possible.
I just checked the lobby DB, and it seems there are still 1000 people without a migrated bcrypt password.
Once we're going to remove the support for those passwords, we're going to need to remove the accounts as well...
We should probably add this to the lobby info text in case we can't bump this version string.

@ssoloff
Copy link
Member

ssoloff commented Jan 12, 2018

Regarding this task:

Submit Instants over the network instead of using Date

This was part of the bigger issue of how RMI methods are enumerated. One thing we should look into is making the method "sorting" a little more forgiving so it doesn't break compatibility to add new remote methods 99% of the time. That will go a long way to ease maintenance until a new network protocol is in place.

We might also want to consider running a parallel lobby during this time that is compatible with the new method sorting for pre-release users. IIRC, @DanVanAtta set up a second lobby instance for testing someplace; don't remember what happened to that.

@ssoloff
Copy link
Member

ssoloff commented Jan 12, 2018

This may be partially covered by some of the above tasks, but what about removing as many deprecated methods/types as possible?

@ssoloff
Copy link
Member

ssoloff commented Jan 12, 2018

What about changing the task "Rename all m_* variables" to "Fix all Checkstyle naming violations"? That will cover packages and types (in addition to fields) that we haven't been able to rename due to save game compatibility.

@ron-murhammer
Copy link
Member Author

@ssoloff Feel free to update it. Though we need to be careful with package names as some are referenced in the map XML.

@ssoloff
Copy link
Member

ssoloff commented Jan 12, 2018

@ron-murhammer Good point. I updated it to read "Fix all Checkstyle naming violations (minus those that would break map XML compatibility)".

@DanVanAtta
Copy link
Member

Incompatible releases are really disruptive in TripleA as games can often be played out over months. It's a pain for a number of other reasons, I'll skip the details though.

What can we do from an architecture point of view to reduce the need for incompatible releases? I think those items should be some of our primary objectives/goals. Making incompatible releases more rare will help everyone. At the same time the compatibility problem is one of the more brittle aspects in the tripleA code base - would be a great benefit to develop to have confidence that any given update would not break network, or break save games.

@RoiEXLab
Copy link
Member

@DanVanAtta
.@ssoloff Should still have his savegame proxy branch somewhere around, so that reduces the amount of incompatible savegames there.
While we're already there, we could also use this chance to refactor the attachment classes and use interfaces instead of reflection.
I believe an important goal should be to fix the netcode which is causing the most compatibility issues at this point, the pseudo RMI does completely break when adding new methods to it.

@DanVanAtta
Copy link
Member

@RoiEXLab IIRC the attachment classes are now looked up by name instead of reflection: https://github.com/triplea-game/triplea/blob/master/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java

@ssoloff also IIRC correctly, the proxy framework we tried to use had a fatal circular dependency. Would that be a problem with a more classic proxy done as a one-time effort? I do remember that approach allowed for GameData references to be removed.

Regardless of whether we use serialization proxy or not - I'm starting to be pretty convinced that save game compatibility should be the goal of next incompatible release. That would give us a lot of flexibility going forward and make it much less burdensome when we require players to update.

@ssoloff
Copy link
Member

ssoloff commented Jan 16, 2018

IIRC correctly, the proxy framework we tried to use had a fatal circular dependency

@DanVanAtta Yes. The first attempt was nice because it was basically a drop-in replacement that didn't require many changes to existing code. But it ultimately failed because there is a fundamental restriction in the Java serialization framework that prevents circular references between proxies.

The second attempt abandoned the proxies as we typically think of them and went with a more memento-based approach. That required a lot of plumbing changes in existing code to be able to reconstruct an object graph. However, there were still some circular reference issues, but they appeared to be resolvable via some refactoring. Unfortunately, the refactoring itself would have introduced compatibility issues, and that's when I decided to abandon the effort.

After going through this twice, I'm thinking that taking a "let's introduce save game compatibility" tack a third time may well fail again. There needs to be some fundamental redesign in the engine. I think using the problems we discover while trying to isolate the persistence functionality should drive changes in the engine. However, my gut's telling me that may take longer than one release cycle.

And I didn't even get to the issues I expected to run into when I tried to proxy the IExecutable stuff... 😄

@RoiEXLab
Copy link
Member

@DanVanAtta

@RoiEXLab IIRC the attachment classes are now looked up by name instead of reflection

Yes that's correct, but I mainly meant the developer site of things. On those attachment objects we are currently calling getters and setters of certain fields via reflection.
Ideally we'd have a map for each attachment type containing objects implementing some sort of attachmentproperty interface that defines a setter and a getter method (and perhaps some overloads for convenience) as values, with their respective names as keys.
Ideally the package name for the attachment types as specified in map xmls wouldn't be necessary either. There shouldn't be multiple attachment types with the sane class name anyways

@RoiEXLab
Copy link
Member

Not necessarily a breaking change, but it would be useful to change the bots to try to reconnect to the lobby in exponential intervals, I.e. Retry in 10 seconds, retry in 100 seconds, thousand, tenthousand etc. Or simply double the value each time.

@DanVanAtta
Copy link
Member

On those attachment objects we are currently calling getters and setters of certain fields via reflection.

Can that be updated without a breaking change?

There needs to be some fundamental redesign in the engine. I think using the problems we discover while trying to isolate the persistence functionality should drive changes in the engine. However, my gut's telling me that may take longer than one release cycle.

We may be doomed if we must always solve this in one release cycle but at the same time cannot. IMO it's worthwhile looking at this in more detail. For the moment, updating variable names and releasing one new future does not yet seem super valuable for breaking everyone's save games. If we stop breaking save games, most of the incompatible pain goes away, players only need to download an updated client and pick up any games they had going.

@RoiEXLab
Copy link
Member

@DanVanAtta

Can that be updated without a breaking change?

Might be possible, but this wouldn't allow us to do the very much needed refactoring.

@DanVanAtta
Copy link
Member

@RoiEXLab @ssoloff @ron-murhammer this is an important topic, we need to get to consensus on this.

Breaking save game compatibility is a major deal, in the past participation has dropped after breaking changes (confusion on lobby, fact that most players remain in old, the mutiple versions to manage).

Old jars was meant to address this, but:

  • many players often did not realize old jars would support the use-case, so old games were launched anyways.
  • old jars did not work in tandem with map updates, on some releases it just does not work

TripleA is a bit unique in that save games can be played out over a few months. If we removed this problem, we could do incompatible releases with much less impact. It's not ideal, but it's okay if the game engine forces an update and then a few minutes later be playing. We could do incompatible releasees every few months, maybe every month. But it's another issue to manage, as a user save game to engine version compatibility over a few months, potentially losing a few games when the old lobby goes away.

This is why I think it makes sense to solve this problem, then incompatible releases becomes much easier down the road and we can make many other changes without needing to wait for the next incompatible release cycle (and again impacting players with older save games that they can no longer easily play)

It's a challenge, but it would mean we would have a goal to fix this problem, perhaps we can try to find something that would work and split it up between us. At the same time queuing up a few useful features to take advantage of the incompatible release would make sense as well. What is everyone else thoughts, is save game compat just not feasible to solve? Will we be locked in this cycle of impacting users over weeks, maybe longer when we release new non-compatible versions (which is why we do not do it very often)?

@simon33-2
Copy link
Contributor

My wish list for a breaking release would be to include:

I cannot see how anything in the OP is more important than the first one at least and probably the second one too. FWIW.

@prastle
Copy link
Contributor

prastle commented Feb 17, 2018

@DanVanAtta Everything you said above is basically true. I do not think you will ever solve this easily. But considering we have peeps playing from a to z engines it would probably be best to have at least have one breaking release every 6 months at the rate of your releases. Just my humble opinion. This at least keeps everyone on the same rough engine page. Which allows easier bug diagnosis. Just my two cents.

@ssoloff
Copy link
Member

ssoloff commented Oct 2, 2018

I know we've talked about this before, but, in light of what happened with infrastructure, what's the plan for pushing hotfixes to the current stable during the next 3-6 months? Are we going to create a dedicated branch for 1.9?

@ron-murhammer
Copy link
Member Author

Ideally, we don't need any :)

In reality, we'll create a branch off of 12226 and make any hotfixes. Then figure out how to deploy those to the bots.

@prastle
Copy link
Contributor

prastle commented Oct 2, 2018

YUP!

@prastle
Copy link
Contributor

prastle commented Oct 2, 2018

Yes I will get yelled at but as a side note (and off topic) If easy hosting was ever developed we would never need bots... ;)

@ssoloff
Copy link
Member

ssoloff commented Oct 2, 2018

Then figure out how to deploy those to the bots.

While it's fresh in my mind... We still might need to create a dedicated branch at that point so we can enable releases from it. Right now, only builds from master produce a release. .travis.yml would have to be updated to include the additional branch.

@ssoloff ssoloff removed their assignment Oct 2, 2018
@RoiEXLab
Copy link
Member

RoiEXLab commented Oct 2, 2018

Not sure if it has been noted related to this topic before, but ideally we'd get rid of the MD5Crypt dependency altogether.
Lobby accounts that are still not migrated are probably "dead" and would be removed from the db. I don't know the numbers, but it would be nice to know how many would be removed in advance.
The next steps would include features that rely on MD5Crypt for some sort of encryption.
Those should be eliminated as well and replaced with a proper encryption algorithm.

Also we want to drop a version number and work on fixing the auto update notification mechanism

@RoiEXLab RoiEXLab removed their assignment Oct 2, 2018
@prastle
Copy link
Contributor

prastle commented Oct 2, 2018

If they have not migrated yet I doubt they ever will. I agree that they are "dead".

@DanVanAtta
Copy link
Member

Is the first-post list current @ron-murhammer ?

It seems like we may still need to do two things:

  1. confirm what to include for a next non-compatible
  2. confirm branching strategy

I can be persuaded otherwise, though it seems like we solve many problems now and mostly forever when we fix compatibility constraints. It seems like our focus should be there. Those three areas are:

  • save game compatibility
  • networking
  • map parsing

@ron-murhammer
Copy link
Member Author

Yeah, its current. I think those are good areas to try and tackle but they do require probably significant effort so would need someone to dedicate cycles to. Feel free update the first post with your thoughts.

@DanVanAtta
Copy link
Member

Thanks. With luck networking compatibility updates are far less impactful than they once were.

For save-game compatibility changes, are these the only two items that would break that?

  • Fix all Checkstyle naming violations like m_* (minus those that would break map XML compatibility)
  • Move types so marked to new packages that couldn't be previously moved due to save game and/or network compatibility

I'm thinking it'd be most worthwhile for us to pick one of those three areas of non-compatibility and try to resolve one on major upgrades. Networking I think has a path forward and progress is being made. That mostly leaves save-game + map parsing.

Very much devil's advocate, if we don't tackle a more major initiative, will dev coding conveniences be worthwhile to the community/user-base? I'm thinking of the many notable examples where save-games have a very long shelf-life. In terms of highest impact, my thinking is we should tackle save-game compatibility.

@prastle
Copy link
Contributor

prastle commented Oct 2, 2018

I think we just do a 2016. Keep two lobbys up.
(opps we are) and give them 6 months warning
save game be dammed!

@DanVanAtta
Copy link
Member

@ssoloff we could go whole hog on serialization proxy, non-backward compatible. That would keep the updates straight forward, we'd drop a lot of GameData references, and that would give us a migration path for replacing java serialization with something else. I could see the alternative to wait on that and just directly migrate to a replacement as well. I'm curious if you think this is the right opportunity to resolve the save game compatibility problem.

@DanVanAtta
Copy link
Member

(asking @ssoloff directly given the extensive work in this area)

@ssoloff
Copy link
Member

ssoloff commented Oct 4, 2018

@DanVanAtta Sure. If we're breaking compatibility, revisiting serialization proxy makes sense.

@DanVanAtta
Copy link
Member

Since we've broken compatibility, it looks like we are on our way. Unless there is a need to re-open, any new topics should probably get their own issue and reference this one as needed.

@DanVanAtta
Copy link
Member

Re-opening, this is a very good planning thread for v1.10 and there are even unresolved topics (this was closed prematurely, my apologies there).

@DanVanAtta DanVanAtta reopened this Feb 19, 2019
@DanVanAtta DanVanAtta changed the title Next Incompatible Release Next Incompatible Release Planning (v1.10) Feb 19, 2019
@DanVanAtta
Copy link
Member

DanVanAtta commented Feb 19, 2019

@ron-murhammer I think this kind of topic is a good fit for Github 'project' tracking. I set up a shell project to get started: https://github.com/triplea-game/triplea/projects/6

I think this issue is perfect for discussing what should land in the 'todo' column and for general 1.10 release planning.

The benefit to the project is that other issues and PR's can be related without needing to remember this issue number or adding a comment to relate them. I do not think it's great practice for us to require edits of comments to see latest status. In one-off cases it works, or if really hacking at the forums, long-term and naturally the person that writes a comment, owns it. Same thing if an issue is created generally. @ron-murhammer it's up to you if you want to keep a check list here or use the project.

Now that we are pretty far into the non-compatible release, it seems we should finish discussing what we intend to be included. To add to the list, a few things to discuss or note:

  • (1) Java 11?
    This would solve our out-of-date Java problem (Version Number Semantics - Replace 'major' with date-month? #4548) without needing to add a feature to detect out-of-date java.
  • (2) JavaFX UI? (for the loading screens)
    The project is a bit stalled, I think we should commit to it now or make the painful choice to remove it. It's not sustainable for us to consider how to make improvements to one UI if we have a long-lived parallel rewrite ongoing in the background.
  • (3) *Begin migrating Lobby Java network serialization to http?
    We can migrate a really large chunk of the lobby network code and convert to http calls. If we are rewriting the lobby UI as part of the loading screens, we can help ourselves along by writing it directly against an http APi. AFAIK it's mainly chat that we probably should not convert to an http interface. It's aggressive, but there is strong synergy benefit if we take this on. In further support of taking this on, we can be quite incremental and would not be forced to migrate everything. The biggest risk IMO exists with getting the new UI to land.
  • (4) Error Reporting
    This is in the home stretch and is potentially the first feature to make use of the new http server
  • (5) Moderator ToolBox 2.0
    I don't think there is any postponing this project, it has been long overdue. Essentially we need to get the mods some updated tools so they don't have to be part-time dev's to keep the trolls out. This also would solve the latent need to have database CLI access to run moderator related queries.
  • (6) Move Login to Https ?
    A number of our compatibility notes relate to old password login. We will need/want authentication for moderator toolbox. We solve a number of technical debt problems and potentially increase our security by going to a design that does not require password decryption.

On my radar has been match challenges (https://github.com/triplea-game/triplea/projects/3). To get there we need an http server, a pattern for interfacing with it, and lots of DB work as we start to do a lot more persistence. In that context, error-reporting has been good for proving a way to do client-server communication, moderator toolbox which I've started recently is intended to break in the DB layer to be able to scale for more complexity.

@prastle
Copy link
Contributor

prastle commented Feb 19, 2019

Above all points have my vote @DanVanAtta just my noob two cents Pras

@DanVanAtta
Copy link
Member

Latest release planning is in this thread: #5025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion team communication thread, meant for coordination and decision making
Projects
None yet
Development

No branches or pull requests

6 participants