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-20697] Ordered fix: prevent unfixed page from being deallocated #488

Merged
merged 5 commits into from Feb 10, 2017

Conversation

ribram
Copy link
Contributor

@ribram ribram commented Feb 9, 2017

http://jira.cubrid.org/browse/CBRD-20697

This issue is that vacuum deallocated pages unfixed by ordered fix. Active worker wants to advance during heap scan from a page to next page. Next page has higher priority, so current page must be unfixed. A vacuum worker fixes page and deallocates it before active worker fixes it again.

@ribram ribram added the bug label Feb 9, 2017
@ribram ribram self-assigned this Feb 9, 2017
@razvanradulescu
Copy link
Contributor

I agree with the fix

ATOMIC_INC_32 (&bufptr->avoid_dealloc_cnt, -1);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have manage mutexes for hash_anchor and bcb, and would like propose the change as follows:

  for (i = 0; i < saved_pages_cnt; i++)
    {
      if (ordered_holders_info[i].prevent_dealloc)
        {
          /* we need to remove prevent deallocate. */
          PGBUF_BUFFER_HASH *hash_anchor = &pgbuf_Pool.buf_hash_table[PGBUF_HASH_VALUE (&ordered_holders_info[i].vpid)];
          bufptr = pgbuf_search_hash_chain (hash_anchor, &ordered_holders_info[i].vpid);

          if (bufptr == NULL)
            {
              /* oops... no longer in buffer?? */
              assert (false);
              pthread_mutex_unlock (&hash_anchor->hash_mutex);
              continue;
            }

          if (bufptr->avoid_dealloc_cnt <= 0)
            {
              /* oops... deallocate not prevented */
              assert (false);
            }
          else
            {
              ATOMIC_INC_32 (&bufptr->avoid_dealloc_cnt, -1);
            }

          PGBUF_BCB_UNLOCK (bufptr);
        }
    }

We may have to also confirm the case: bufptr == NULL
This case means that bcb was victimized by the other. It is very unlikely, but I'm not 100% sure.
Can you confirm it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have made the required changes :).

@ribram ribram merged commit 0aac543 into CUBRID:develop Feb 10, 2017
@ribram ribram deleted the CBRD-20697 branch February 10, 2017 09:10
@eseokoh eseokoh modified the milestone: banana pie May 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants