Skip to content

[Fix][Transaction] Optimize log when create transaction systemTopic#14911

Closed
liangyepianzhou wants to merge 1 commit intoapache:masterfrom
liangyepianzhou:optimize_log_create_txn_systemTopic
Closed

[Fix][Transaction] Optimize log when create transaction systemTopic#14911
liangyepianzhou wants to merge 1 commit intoapache:masterfrom
liangyepianzhou:optimize_log_create_txn_systemTopic

Conversation

@liangyepianzhou
Copy link
Contributor

Fix #14790

Motivation

When users use admin to get status of transaction system topic,they will get a confused error.

Modification

Optimize the error msg.

Motivation

Explain here the context, and why you're making that change. What is the problem you're trying to solve.

Modifications

Describe the modifications you've done.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

### Motivation
When users use admin to get status of transaction system topic,they will get a confused error
### Modification
Optimize the error msg
if (isTransactionSystemTopic(topicName)) {
String msg = String.format("Can not create transaction system topic %s", topic);
String msg = String.format("Only has metadata, used for transactionCoordinator load balance, "
+ "never need to be created, for transaction system topic %s", topic);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah...it's funny.
@Anonymitaet Could you help review this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liangyepianzhou Sorry, I don't quite understand the meaning. Could you please explain a bit more?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, the current situation is that we have some system topics for lookup that only need to create metadata.
Then the user used admin to query this topic, but our error made the user confused and raised the issue.

Copy link
Contributor

@gaozhangmin gaozhangmin Mar 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore creation of transaction system topic, no need to create. @Anonymitaet. I think the meaning is.

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Topic stats cannot work with transaction_coordinator_assign topic

4 participants