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

feat: enhance eventbus with service_event #36

Merged
merged 1 commit into from May 20, 2024

Conversation

qiankunli
Copy link
Contributor

No description provided.

eventbus/mysql/eventbus.go Outdated Show resolved Hide resolved
eventbus/mysql/eventbus.go Outdated Show resolved Hide resolved
eventbus/mysql/eventbus.go Outdated Show resolved Hide resolved
if len(events) == 0 {
return nil
} else if eventPO.Event.SendType == dddfirework.SendTypeLaxFIFO {
// 当期的策略是,哪怕前序event 在重试中,也还是init,除非超过重试次数status=failed。若想忽略重试这一点,可以check下 precedingServiceEvent.retryCount
Copy link
Collaborator

Choose a reason for hiding this comment

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

重试中(甚至包括运行中)都归到init,有点反直觉,也不利于追踪,我看区分出来并不难?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

retry_count > 1即表示重试。独立一个状态也可以,不过scanEvents 那个地方就略微复杂些

Copy link
Collaborator

Choose a reason for hiding this comment

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

那要不ServiceEventStatusInit换个名字,pending如何

eventbus/mysql/eventbus.go Show resolved Hide resolved

// ServiceEventPO 记录service对event的处理情况
type ServiceEventPO struct {
ID int64 `gorm:"primaryKey;autoIncrement"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

新机制不需要一个数字自增id了,要不删了?主键就用idx_service_event_id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是目前略微玄学的一点,不要自增id,TestEventBusConcurrent 中ConsumeConcurrent=1时,无法保证 事件消费的有序性,可能是scanEvent 得到的顺序可能不同。

Copy link
Contributor Author

@qiankunli qiankunli May 6, 2024

Choose a reason for hiding this comment

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

原因找到了,在getScanEvents 有下面一段逻辑

eventOffset := int64(0)
lastServiceEvent := &ServiceEventPO{}
if err := e.db.Where("service = ?", e.serviceName).
  Order("event_id").Last(lastServiceEvent).Error; err != nil {
    if !errors.Is(err, gorm.ErrRecordNotFound) {
	return nil, err
    }
}
if lastServiceEvent.EventID > 0 {
    eventOffset = lastServiceEvent.EventID
}

在去掉id 自增主键以 service+event_id 为主键之后,查询sql是

 SELECT * FROM `ddd_domain_service_event` WHERE service = 'test_concurrent' ORDER BY event_id,`ddd_domain_service_event`.`service` DESC LIMIT 1;

这与 下面sql的执行结果是截然不同的(下面的sql 给结果是对的),估计是order by 时event_id 在前导致无法使用主键索引(主键索引是service+event_id)

 SELECT * FROM `ddd_domain_service_event` WHERE service = 'test_concurrent' ORDER BY `ddd_domain_service_event`.`service`,event_id DESC LIMIT 1;

此外,gorm First 和 Last 默认会给order 带上主键。
也因此,使用service+event_id 索引 + first/last 语句时,就不能额外再指定 Order("event_id"),或者说必须保持gorm order里的内容必须与主键索引的创建顺序一致(或者说不能相互影响),否则会影响代码逻辑正确性,这一细节很容易被遗忘

Copy link
Collaborator

Choose a reason for hiding this comment

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

checkPrecedingEvent会保证有序,db返回无序也没关系

Copy link
Contributor Author

Choose a reason for hiding this comment

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

但是对 getScanEvents 逻辑是有影响的。gorm 的first 和 last 语句(哪怕指定了order 字段)会自动带上对主键字段的排序,进而导致有时first 和 last返回的记录跟代码直觉不符,所以个人感觉不必为了省个字段而去掉主键id。

eventbus/mysql/eventbus.go Outdated Show resolved Hide resolved
@qiankunli qiankunli force-pushed the feat/enhance-eventbus branch 4 times, most recently from 8f13c4d to 25817f6 Compare May 6, 2024 09:26
eventbus/mysql/po.go Outdated Show resolved Hide resolved
eventbus/mysql/po.go Outdated Show resolved Hide resolved
eventbus/mysql/po.go Outdated Show resolved Hide resolved
eventbus/mysql/eventbus.go Show resolved Hide resolved
eventbus/mysql/eventbus.go Show resolved Hide resolved
eventbus/mysql/po.go Show resolved Hide resolved
eventbus/mysql/po.go Show resolved Hide resolved
@qiankunli qiankunli force-pushed the feat/enhance-eventbus branch 3 times, most recently from 72befb7 to 8c6de28 Compare May 7, 2024 01:20
@liqiankun1111 liqiankun1111 merged commit de509f4 into bytedance:main May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants