Skip to content

Commit

Permalink
chore: remove transactional annotation (#6492)
Browse files Browse the repository at this point in the history
#### What type of PR is this?
/kind cleanup
/area core
/milestone 2.19.x

#### What this PR does / why we need it:
移除事务注解避免对索引创建产生影响,原因参考改动中的方法注释

**其中一点特别注意:**
在执行 `client.create(name, data)` 方法后,会尝试进行 `indexer.indexRecord` 操作。但 indexRecord 可能会因唯一索引中存在重复键而导致 indexRecord 失败,索引创建也会随之失败。为确保索引与数据的一致性,此时应回滚由 `client.create(name, data)` 对数据产生的影响,因此除非找到更佳的一致性问题解决方案,否则暂时不能移除此处的手动事务操作。

```java
 return client.create(name, version, data)
                .map(updated -> converter.convertFrom(type, updated))
                .doOnNext(extension -> indexer.indexRecord(convertToRealExtension(extension)))
                .as(transactionalOperator::transactional);
```
将变更传递给 extension watcher 是在 `doCreate` 或 `doUpdate` 成功之后才会被处理,因此这里的事务回滚不会对 watcher 造成影响

#### Does this PR introduce a user-facing change?
```release-note
None
```
  • Loading branch information
guqing authored Aug 22, 2024
1 parent d8ec34b commit f9615d0
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.springframework.data.domain.Sort;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
Expand Down Expand Up @@ -147,7 +146,6 @@ public Mono<User> grantRoles(String username, Set<String> roles) {
}

@Override
@Transactional
public Mono<User> signUp(User user, String password) {
if (!StringUtils.hasText(password)) {
throw new IllegalArgumentException("Password must not be blank");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,44 @@ public IndexedQueryEngine indexedQueryEngine() {
return this.indexedQueryEngine;
}

/**
* <p>Note of transactional:</p>
* <p>doSomething does not have a transaction, but methodOne and methodTwo have their own
* transactions.</p>
* <p>If methodTwo fails and throws an exception, the transaction in methodTwo will be rolled
* back, but the transaction in methodOne will not.</p>
* <pre>
* public void doSomething() {
* // with manual transaction
* methodOne();
* // with manual transaction
* methodTwo();
* }
* </pre>
* <p>If doSomething is annotated with &#64;Transactional, both methodOne and methodTwo will be
* executed within the same transaction context.</p>
* <p>If methodTwo fails and throws an exception, the entire transaction will be rolled back,
* including any changes made by methodOne.</p>
* <p>This ensures that all operations within the doSomething method either succeed or fail
* together.</p>
* <p>This example advises against adding transaction annotations to the outer method that
* invokes {@link #update(Extension)} or {@link #create(Extension)}, as doing so could
* undermine the intended transactional integrity of this method.</p>
*
* <p>Note another point:</p>
* After executing the {@code client.create(name, data)} method, an attempt is made to
* indexRecord. However, indexRecord might fail due to duplicate keys in the unique index,
* causing the index creation to fail. In such cases, the data created by {@code client
* .create(name, data)} should be rolled back to maintain consistency between the index and
* the data.
* <p><b>Until a better solution is found for this consistency problem, do not remove the
* manual transaction here.</b></p>
* <pre>
* client.create(name, data)
* .doOnNext(extension -> indexer.indexRecord(convertToRealExtension(extension)))
* .as(transactionalOperator::transactional);
* </pre>
*/
@SuppressWarnings("unchecked")
<E extends Extension> Mono<E> doCreate(E oldExtension, String name, byte[] data) {
return Mono.defer(() -> {
Expand All @@ -325,6 +363,9 @@ <E extends Extension> Mono<E> doCreate(E oldExtension, String name, byte[] data)
});
}

/**
* see also {@link #doCreate(Extension, String, byte[])}.
*/
@SuppressWarnings("unchecked")
<E extends Extension> Mono<E> doUpdate(E oldExtension, String name, Long version, byte[] data) {
return Mono.defer(() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import run.halo.app.infra.exception.DuplicateNameException;
Expand Down Expand Up @@ -61,7 +60,6 @@ public Mono<ExtensionStore> update(String name, Long version, byte[] data) {
}

@Override
@Transactional
public Mono<ExtensionStore> delete(String name, Long version) {
return repository.findById(name)
.flatMap(extensionStore -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.springframework.core.io.Resource;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.StringUtils;
import org.springframework.web.server.ServerWebInputException;
import reactor.core.Exceptions;
Expand Down Expand Up @@ -136,7 +135,6 @@ public Mono<Resource> download(Backup backup) {
}

@Override
@Transactional
public Mono<Void> restore(Publisher<DataBuffer> content) {
return Mono.usingWhen(
createTempDir("halo-restore-", scheduler),
Expand Down

0 comments on commit f9615d0

Please sign in to comment.