Revert mars state machine changes

[cheenamalhotra]

Fixes #1234

• Test added to reproduce failures with change from Rework MARSConnection state machine #933.

P.S. we've been having this issue in pipelines too, they're failing a lot more recently.
At this point for 4.0 GA it's safest to revert it.

[Wraith2]

Reverting for 4.0 release and then putting back in or just reverting and leaving out entirely?

[cheenamalhotra]

Reverting for 4.0 release and then putting back in or just reverting and leaving out entirely?

We can accept again with fix incorporated for failing test.

[Wraith2]

Who will be doing that? If we want to evolve the managed implementation then this mars code needs to change but it's a big complex job and waiting for me to work it all out doesn't seems sensible.

[cheenamalhotra]

Who will be doing that? If we want to evolve the managed implementation then this mars code needs to change but it's a big complex job and waiting for me to work it all out doesn't seems sensible.

Sure, currently it's not in our plan as it's not highly requested change and is an improvement that needs rework to fix the issues.
We will look into it as an enhancement when we have capacity post GA. You can create an issue to be able to track it for future.

[Wraith2]

I thought improving managed performance was a goal. I worked on it on the basis that it was in furtherance of that goal. Without understanding this particular piece of code you can't untangle other aspects of the managed implementation.

If it's not something that anyone wants then it was a waste of time and it would have been good to know that.

[cheenamalhotra]

Managed performance improvement is certainly our goal, but whether the proposed PR is going to be reconsidered depends on how much we need to dig down in this change in order to fix this bug. We will be considering other design improvements too if we come up something better, that’s why merging this exact change is uncertain right now.

We weren’t expecting this bug of course, that’s why we merged the proposal, but now we may have to reconsider other options to improve performance in this domain.

[Wraith2]

We will be considering other design improvements too if we come up something better, that’s why merging this exact change is uncertain right now.

If someone can provide a better solution that sounds good to me. Unfortunately I don't believe that will happen based on my experience working on this library so far. Even important user affecting and often reported issues like async string perf sit untouched for months to years. To come up with something better someone would have to make the effort and take the time to do so, and that just won't happen.

[Wraith2]

Now that 4.0 has shipped can this reversion be considered again. The intent of the original fix is still good it just needs attention on edge cases that wasn't available before.

[JRahnama]

@Wraith2, we are interested in the change/PR, but cannot merge it again without fixing the issue it caused before. If you can address the issue we will gladly take it back. Please provide enough tests for edge cases as well. We have also added pipelines for net 6 testing.

[Wraith2]

All on my own again then.

[JRahnama]

All on my own again then.

Well, we ,as a team, and I personally appreciate your contribution. You have been a great help to us. If there is any part that you need us to look into, ping me or other team members and we will jump in. Currently, all other team members are busy with EOY tasks and reports and some other features to add into the driver.

[JRahnama]

@Wraith2 @DavoudEshtehari is on vacation and will be back next week. Will you be interested in having a call and go through the changes and failures of this PR and come up with a plan sometime next week?

[Wraith2]

Sure. Drop me an email invite of when's best for you guys.