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

[CBRD-20185] File manager redesign #197

Merged
merged 351 commits into from
Nov 23, 2016
Merged

[CBRD-20185] File manager redesign #197

merged 351 commits into from
Nov 23, 2016

Conversation

ribram
Copy link
Contributor

@ribram ribram commented Aug 9, 2016

This is work in progress. But I'll open the PR so we can start a conversation during development.

@eseokoh
I have made some changes to log manager and system operations. The basic idea is to extend the commit system operation to handle complex cases of undo, compensate and run postpone. This is an older idea I had, but that was never implemented. I think the time has come, since I really need it for file manager redesign. The alternative is yet another hackish ways of handling some complex operations on file header and tables (hackish ways that in the end may prove even harder to implement than this).

Please review the changes in the commit fe605f0.

@xkyu xkyu changed the title File manager redesign [CBRD-20185] File manager redesign Aug 26, 2016
@cubridci
Copy link

Build comment file:

File: src/storage/disk_manager.c (1 incorrect indentation(s) found)

  3021        + cursor->offset_to_unit * DISK_STAB_UNIT_BIT_COUNT + cursor->offset_to_bit == cursor->sectid);

Please run 'indent -l120 -lc120' to correct the indentation


1 similar comment
@cubridci
Copy link

Build comment file:

File: src/storage/disk_manager.c (1 incorrect indentation(s) found)

  3021        + cursor->offset_to_unit * DISK_STAB_UNIT_BIT_COUNT + cursor->offset_to_bit == cursor->sectid);

Please run 'indent -l120 -lc120' to correct the indentation


@cubridci
Copy link

Build comment file:

File: src/storage/disk_manager.c (2 incorrect indentation(s) found)

  4666  {
  4668  }

Please run 'indent -l120 -lc120' to correct the indentation


@cubridci
Copy link

Build comment file:

File: src/storage/file_manager.c (1 incorrect indentation(s) found)

 14232        || (file_extdata_is_empty (part_table) && VPID_ISNULL (&part_table->vpid_next)));

Please run 'indent -l120 -lc120' to correct the indentation


@eseokoh
Copy link
Contributor

eseokoh commented Nov 22, 2016

int
disk_auto_expand (THREAD_ENTRY * thread_p)
{   
  int error_code = NO_ERROR;
  
  /* todo: we cannot expand the volumes unless we have a transaction descriptor. we might allocate a special tdes for
   *       auto-volume expansion thread, similar to how vacuum works. otherwise, it can be limited to extend last
   *       volume only.
   * for now, do nothing. we'll think about it later.
   */

  return error_code;
}

I think to extend only the last volume would be enough and DBAs expect to extend a volume and create a new one when the last reaches the maximum size.

@cubridci
Copy link

Build comment file:

File: src/storage/file_manager.c (1 incorrect indentation(s) found)

  1755       *page_out = NULL;		/* make it sure for an error */

Please run 'indent -l120 -lc120' to correct the indentation


@ribram ribram merged commit cc36832 into CUBRID:develop Nov 23, 2016
@ribram ribram deleted the file-manager branch November 23, 2016 10:59
@eseokoh eseokoh modified the milestone: banana pie May 12, 2017
ribram added a commit to ribram/cubrid that referenced this pull request May 21, 2021
* [SC-301] Separate file for executing click counter (CUBRID#170)

Isolate click counter implementation from the rest of query executor file in preparation for the story fix.

Add query_click_counter files.

- header is shared with query_executor.c
- implementation in unit file

Side-effects:

- Move qexec_prune_spec declaration from query executor unit to header; forward access_spec_node in header.
- Replace xasl_state argument of qexec_execute_selupd_list with val_descr. xasl_state is local to query_executor unit.

* [SC-301] Rename and add query namespace to click counter functions (CUBRID#171)

- Add query namespace (causes many indent differences)
- Name functions more intuitively
- Replace macro with a functions for readability

* [SC-301] Allow empty sysop for "commit replicated" (CUBRID#197)

**Context**
System operations can sometimes be ended, and committed. Take next scenario:
```c
log_sysop_start ();
// do ops that sometimes are no-op; e.g. INCR(NULL);
log_sysop_commit ();
```
Sysop commit is a no-op if no changes were done during the system operation.

However, for logical undo, compensate and run postpone, having an empty system operation is not acceptable (an logged action was mandatory). Therefore, there is a safe-guard that checks sysop is not empty for all cases except normal commit.

**Issue**
In click counter, the sysop commit is being replaced with sysop "commit replicated"
```c
log_sysop_start ();
// do ops that sometimes are no-op; e.g. INCR(NULL);
log_sysop_commit_replicated ();
```
A new sysop commit type triggers the safe-guard. However, this type of sysops do not have the same requirement as logical undo, compensate, run postpone.

**Fix**
Update the safe-guard to accept the case.

Add new helper functions to make code more readable.

* [SC-301] Check tran index and early out if null in logtb_set_current_user_active (CUBRID#198)

New situations that call session_state_destroy do it after freeing transaction descriptor. session_state_destroy calls logtb_set_current_user_active and a crash occurs if the transaction descriptor has been freed.

Normally, transaction and session management should be decoupled. In some cases, session outlives the transactions (e.g. connection down), in others transaction outlives session (csql shutdown sequence first destroys sessions, then transaction).

This is a quick-fix to make calling session_state_destroy safe without risking crashes until transactions/sessions are properly managed.

* [SC-301] Assign tdes of transaction table for applying sub-transaction (CUBRID#199)


* Applying click counter changes automatically generates an MVCCID. System transaction descriptors cannot be assigned an MVCCID. As consequence, applying subtransaction is assigned a tdes from transaction table instead of a system tdes.  MVCCID is completed immediately after applying the changes.
* Applying click counter change also automatically locks modified objects. Using instant locking to immediately free the locks after applying all changes.
* After applying all stream entries and before releasing the transaction, unlock all locks and free the session state.
* locator_repl_end_tran is not used because transaction is never actually committed. All changes are done as part of system operations that are committed immediately after.

* [SC-301] Use subtransactions to replicate click counter (CUBRID#201)

Use cubtx::subtransaction in execute_click_counter instead of manually handling sysops and replication.

* [SC-301] Early out when no click counter is incremented (CUBRID#200)

Click counters can sometimes be no ops. For instance INCR(NULL) is skipped and doesn't do any increments. Add an early out if all click counter operations are skipped.\
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants