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 15848 #16435

Merged
merged 4 commits into from
May 28, 2024
Merged

fix 15848 #16435

merged 4 commits into from
May 28, 2024

Conversation

jiangxinmeng1
Copy link
Contributor

@jiangxinmeng1 jiangxinmeng1 commented May 28, 2024

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #15848 https://github.com/matrixorigin/MO-Cloud/issues/3157 https://github.com/matrixorigin/MO-Cloud/issues/3279 https://github.com/matrixorigin/MO-Cloud/issues/3339

What this PR does / why we need it:

Not set object created by CN as 1PC node

@mergify mergify bot added the kind/bug Something isn't working label May 28, 2024
@matrix-meow matrix-meow added the size/XS Denotes a PR that changes [1, 9] lines label May 28, 2024
mergify bot pushed a commit that referenced this pull request May 28, 2024
Not set object created by CN as 1PC node

Approved by: @XuPeng-SH, @sukki37
@sukki37
Copy link
Contributor

sukki37 commented May 28, 2024

/review

Copy link

PR Review 🔍

⏱️ Estimated effort to review [1-5]

2, because the PR involves a single line change in the logic of object creation, which is straightforward to understand. However, understanding the impact of this change on the overall system might require deeper knowledge of the system's architecture and the specific function of the modified code.

🧪 Relevant tests

No

⚡ Possible issues

Possible Bug: Changing the boolean parameter in CreateNonAppendableObject from true to false might have unintended effects elsewhere in the system if other parts of the codebase rely on the previous behavior. This needs thorough testing to ensure no other functionalities are adversely affected.

🔒 Security concerns

No

Code feedback:
relevant filepkg/vm/engine/tae/txn/txnimpl/table_space.go
suggestion      

Consider adding error handling after changing the object creation logic. If CreateNonAppendableObject fails, it should be handled appropriately to prevent further issues in the transaction processing. [important]

relevant linespace.nobj, err = space.table.CreateNonAppendableObject(false, new(objectio.CreateObjOpt).WithId(sid))

@mergify mergify bot merged commit 086838a into matrixorigin:main May 28, 2024
17 of 19 checks passed
@aylei aylei mentioned this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working Review effort [1-5]: 2 size/XS Denotes a PR that changes [1, 9] lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants